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

[PropertyAccess] Fixed invalid feedback -> foodback singularization #13618

Merged
merged 1 commit into from Feb 16, 2015

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Feb 7, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #13613
License MIT
Doc PR -

@wouterj
Copy link
Member Author

wouterj commented Feb 7, 2015

@fabpot maybe it's better if fabbot would ignore these 2 files when checking for typos :)

@@ -67,6 +68,7 @@ public function singularifyProvider()
array('emphases', array('emphas', 'emphase', 'emphasis')),
array('faxes', 'fax'),
array('feet', 'foot'),
array('feedback', 'feedback'),
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it a problem to have the same plural and singular?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure what you mean. Feedback is not a coutable noun and by that, it can't be pluralized. See http://english.stackexchange.com/questions/176805/plural-of-feedback

The StringUtil already coped with this well, the only bug was that it was replacing all words with "ee" in it and longer than 3 characters with "oo", turing "feedback" into "foodback".
This PR adds "feedback" as an exception to this replacement statement (we may benefit from a more general fix for this, but I do not consider myself skilled enough to do that).

Copy link
Contributor

Choose a reason for hiding this comment

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

@wouterj I think the question was more related to problems in how property accessor uses singular/plural forms, and possible issues when these forms are the same.

Copy link
Member

Choose a reason for hiding this comment

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

Exactly, I was just wondering if having the same word for a singular and a plural would not lead to issues when using such code. getFeedback() -> plural or singular? $this->feedback -> plural or singular?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think Symfony has to worry about it? (or maybe I'm not fully aware of the usage of this class) Does it matter to Symfony whether getFeedback() is a plural or a singular? It might not be the best choice because of this, but that's a problem of the project imo.

@fabpot
Copy link
Member

fabpot commented Feb 16, 2015

Thank you @wouterj.

@fabpot fabpot merged commit bc50125 into symfony:2.3 Feb 16, 2015
fabpot added a commit that referenced this pull request Feb 16, 2015
…arization (WouterJ)

This PR was merged into the 2.3 branch.

Discussion
----------

[PropertyAccess] Fixed invalid feedback -> foodback singularization

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #13613
| License       | MIT
| Doc PR        | -

Commits
-------

bc50125 Fixed invalid feedback -> foodback singularization
@wouterj wouterj deleted the issue_13613 branch April 3, 2015 08:55
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.

None yet

3 participants