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

fix: pluralize in mistaken word #146

Merged
merged 1 commit into from
Apr 12, 2017
Merged

fix: pluralize in mistaken word #146

merged 1 commit into from
Apr 12, 2017

Conversation

pei0804
Copy link
Contributor

@pei0804 pei0804 commented Apr 11, 2017

bitbucket.org/pkg/inflect
inflect.Pluralize("status") -> status (x)

github.com/jinzhu/inflection 
inflection.Plural("status") -> statuses(◯)

If you use bitbucket.org/pkg/inflect Pluralize, the result that is different from the result you want is returned as described above.
The reason is simply because the method of the specification is just judging with the trailing letters.
As a countermeasure, you receive correct results by using github.com/jinzhu/inflection inflection.Plural.

Also, if you have the necessary test cases suggestions please.

@pei0804 pei0804 changed the title fix: pluralize in part wrong [WIP]fix: pluralize in part wrong Apr 11, 2017
@pei0804 pei0804 changed the title [WIP]fix: pluralize in part wrong fix: pluralize in part wrong Apr 11, 2017
@pei0804 pei0804 changed the title fix: pluralize in part wrong fix: pluralize in mistaken word Apr 11, 2017
@raphael
Copy link
Member

raphael commented Apr 11, 2017

Thank you! I don't think we should be testing the external package but given that one of them is given an incorrect result it may be good to add a test for "status" -> "statuses".

@pei0804
Copy link
Contributor Author

pei0804 commented Apr 12, 2017

We have added a few tests focusing on TableName. Verification please.

@raphael
Copy link
Member

raphael commented Apr 12, 2017

Looks great, thank you!

@raphael raphael merged commit aff18b5 into goadesign:master Apr 12, 2017
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.

2 participants