Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Link and Array ContentTypes in src/DynamicEntry #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lesleydesmet
Copy link

Sometimes the raw data is still an object so the function "getCoarcedfield" has to return the fields.

@shieldo
Copy link
Contributor

shieldo commented Feb 6, 2018

@lesleydesmet Thanks for this PR! Would it be possible to add to the unit test against this class to cover the cases being added here? It'll help be clearer about what the change is doing, and whether it is successful.

private function formatArray($array)
{
if (count($array) == 1) {
return array_first($array);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using the return $array[0] directly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point - array_first seems to be a helper function from Laravel, rather than being in the PHP standard library.

return array_values($array)[0];

would usually be better.

@shieldo
Copy link
Contributor

shieldo commented Apr 12, 2019

Hi @lesleydesmet - I'd really like to include this change if you still feel it's useful. It would be good to have a unit test here - let me know if you need any assistance! Also, as per the comment above, the function array_first is not in the PHP standard library (it's a helper function within Laravel, so maybe that's where this came from?) so that would need to be replaced as well as not everybody will use this library within the context of a Laravel application!

@shieldo
Copy link
Contributor

shieldo commented Apr 15, 2019

To be clear - this PR is not mergeable in its current state, given the use of the array_first function which is not in the PHP standard library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants