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

feat(v2): pluralize posts on tag's page #2263

Merged
merged 4 commits into from
Feb 6, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Feb 1, 2020

Motivation

Adding basic feature to form plural word forms. This is of course a very simple util function, and when localization functionality appears, it needs to be improved, but at the moment it is doing its job perfectly

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Before After
image image

@lex111 lex111 added the pr: new feature This PR adds a new API or behavior. label Feb 1, 2020
@lex111 lex111 requested a review from yangshun February 1, 2020 22:01
@lex111 lex111 requested a review from wgao19 as a code owner February 1, 2020 22:01
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Feb 1, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Feb 1, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 52982ee

https://deploy-preview-2263--docusaurus-2.netlify.com

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

I don't know if pluralize should be part of Docusaurus core utils. I get that we want to fix it for the blog page, but can we just add it to the blog plugin instead of Docusaurus core? I don't want to bloat up the core. The smaller API surface we have, the better for maintainability.

@lex111
Copy link
Contributor Author

lex111 commented Feb 3, 2020

Hmm, yes, apparently I was mistaken with this decision, fixed ✔️

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

LGTM but CI is failing. Not sure why.

@lex111
Copy link
Contributor Author

lex111 commented Feb 3, 2020

Fixed, it seems to me time to rest 🤕

@yangshun yangshun merged commit b7a2ad2 into facebook:master Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants