Skip to content

Conversation

@teohhanhui
Copy link
Contributor

No description provided.

@teohhanhui
Copy link
Contributor Author

Do we also want to export these class constants as container parameters?

So that instead of writing

    resource.book.date_filter:
        parent: api.doctrine.orm.date_filter
        arguments:
          - datePublished: 0 # Dunglas\ApiBundle\Doctrine\Orm\Filter\DateFilter::EXCLUDE_NULL

I can just write

    resource.book.date_filter:
        parent: api.doctrine.orm.date_filter
        arguments:
          - datePublished: %api.doctrine.orm.date_filter.exclude_null%

@sroze
Copy link
Contributor

sroze commented Aug 9, 2015

IMO it's better to have string values in DateFilter constants.

@ogizanagi
Copy link
Contributor

👍 for string values in DateFilter constants

@teohhanhui teohhanhui changed the title Fix DateFilter documentation Use string values in DateFilter null-management constants Aug 11, 2015
@teohhanhui
Copy link
Contributor Author

Are we good?

@sroze
Copy link
Contributor

sroze commented Aug 14, 2015

I'm okay with that. But as there's a BC break, I'm waiting another maintainer's 👍.

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 letting the default value settable with ~?

Copy link
Member

Choose a reason for hiding this comment

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

Ok with Theo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The null value is for using database default (i.e. no special null handling). That has not been changed.

What I'm doing here is correcting the example for excluding null values (view the full file to see the text above it).

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense

@dunglas
Copy link
Member

dunglas commented Aug 14, 2015

👍 it's the good time to break things (before 1.0).
Can you also update the changelog please?

@teohhanhui
Copy link
Contributor Author

Added a line in changelog

@sroze
Copy link
Contributor

sroze commented Aug 17, 2015

Thank you @teohhanhui ! 👍

sroze added a commit that referenced this pull request Aug 17, 2015
Use string values in DateFilter null-management constants
@sroze sroze merged commit 1c88abe into api-platform:master Aug 17, 2015
teohhanhui added a commit to teohhanhui/api-platform-core that referenced this pull request Oct 6, 2015
Since there never was a 1.0.0 beta 4
dunglas added a commit that referenced this pull request Oct 6, 2015
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.

5 participants