Skip to content

Conversation

@tniessen
Copy link
Contributor

Fixes #403

Copy link
Member

Choose a reason for hiding this comment

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

Please, could you move options above value? Also, replace >> with > only.

@rxaviers
Copy link
Member

Hi @tniessen, I left a couple of comments above. Great job so far.

Copy link
Member

Choose a reason for hiding this comment

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

Let's create a similar table for ordinals?

Copy link
Member

Choose a reason for hiding this comment

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

👍

@jzaefferer
Copy link
Contributor

I'd like to see a brief definition for both cardinals and ordinals somewhere in the documentation. More examples would also help.

@jzaefferer jzaefferer mentioned this pull request Mar 16, 2015
@tniessen
Copy link
Contributor Author

@rxaviers @jzaefferer I pushed a commit a few days ago, awaiting reviews.

Copy link
Member

Choose a reason for hiding this comment

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

All table needs the below change.

- plural( 0 )
+ plural( 0, { type: "ordinal" } )

@rxaviers
Copy link
Member

Hi @tniessen, I added a couple of comments. Github doesn't send notifications when new commits are pushed. So, feel free to add a new comment if you have further updates. Thanks so far.

@tniessen
Copy link
Contributor Author

@rxaviers Agree with both, new code is almost identical with your suggestion. Updated as 583ae45.

@rxaviers
Copy link
Member

Thank you @tniessen, one last comment.

@tniessen
Copy link
Contributor Author

@rxaviers Fixed in 7538dec, thank you for reviewing!

src/plural.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

spaces MakePlural.rules[ type ] =

@tniessen
Copy link
Contributor Author

@rxaviers My bad, sorry... c47456f

@rxaviers rxaviers closed this in 13b63ff Mar 20, 2015
@rxaviers
Copy link
Member

@tniessen, no problem. Thanks landed 🎉

@tniessen tniessen deleted the fix-403 branch March 20, 2015 15:25
ashensis pushed a commit to ashensis/globalize that referenced this pull request Mar 17, 2016
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.

Support ordinals

4 participants