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

misc: replace all "Metadatas" with "Metadata" #5871

Merged
merged 8 commits into from
Nov 9, 2021

Conversation

swalahamani
Copy link
Contributor

@swalahamani swalahamani commented Nov 4, 2021

Breaking changes

  • We replaced all API containing metadatas with metadata. This includes but is not limited to:

    • themeConfig.metadatas
    • Layout.Props.searchMetadatas
    • @theme/SearchMetadatas

    If you use any of these APIs, you need to update accordingly.
    Why? Because metadatas is grammatically incorrect. We are not going to pretend like we are grammar masters who made this API with a surprising purpose—we are just wrong, and now we fix it.

Motivation

The plural form of Metadata is Metadata itself and not Metadatas.
Please refer: Wordhippo

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

N/A as this is just a spelling correction.

Related PRs

N/A

The plural form of Metadata is Metadata itself and not Metadatas.
@facebook-github-bot
Copy link
Contributor

Hi @swalahamani!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@netlify
Copy link

netlify bot commented Nov 4, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 27c336f

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6187a3d621201200070a4a0b

😎 Browse the preview: https://deploy-preview-5871--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Nov 4, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 95
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5871--docusaurus-2.netlify.app/

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 4, 2021
@facebook-github-bot

This comment has been minimized.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Nov 4, 2021

I don't really know... I also have some objection to "metadatas" but we use it here and there. I'm going to use this chance to run a global search & replace

@slorber What do you think? I think there are only two worlds: (a) we close this PR and do nothing and pretend we are grammar masters and (b) we fix all "metadatas" including the themeConfig API and release this as BC. I don't think fixing this in the docs without fixing it in the code is worth it and just leads to more confusion.

@slorber
Copy link
Collaborator

slorber commented Nov 4, 2021

It's my fault, used to think metadatas was correct 😅

I'm ok to change it everywhere, including a breaking change in theme API considering we'd rather do that before 2.0

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Nov 4, 2021

Actually, I did a random chat with someone else and they said metadatas is acceptable :P

If you have a concept of metadata, and metadatas is an array of that, I think it's perfectly fine
I could see an argument that metadata is a group of metadatums, and metadatas is a group of multiple metadatums

But anyways, fixing it seems more sensible

@facebook-github-bot

This comment has been minimized.

@slorber
Copy link
Collaborator

slorber commented Nov 4, 2021

🤪

@Josh-Cena Josh-Cena added pr: polish This PR adds a very minor behavior improvement that users will enjoy. pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. labels Nov 5, 2021
@Josh-Cena Josh-Cena changed the title docs: update markdown-features-head-metadatas.mdx - fix typo (Metadatas -> Metadata) misc: replace all "Metadatas" with "Metadata" Nov 5, 2021
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

LGTM thanks 👍

@slorber slorber merged commit c541e2d into facebook:main Nov 9, 2021
Josh-Cena added a commit that referenced this pull request Dec 9, 2021
@Josh-Cena Josh-Cena mentioned this pull request Feb 21, 2022
7 tasks
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: breaking change Existing sites may not build successfully in the new version. Description contains more details. pr: polish This PR adds a very minor behavior improvement that users will enjoy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants