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(docs): fix brand name -> gatsby-transformer-{cloudinary,yaml,remark} #27220

Closed
wants to merge 5 commits into from
Closed

fix(docs): fix brand name -> gatsby-transformer-{cloudinary,yaml,remark} #27220

wants to merge 5 commits into from

Conversation

jazibjafri
Copy link

@jazibjafri jazibjafri commented Oct 1, 2020

Description

changes:

  • Gatsby-transformer-cloudinary -> gatsby-transformer-cloudinary (always lowercase)
  • gatsby-transformer-yaml -> gatsby-transformer-yaml (always in codeblocks)
  • gatsby-transformer-remark -> gatsby-transformer-remark (always in codeblocks)

Documentation

Related Issues

#25290 enhancement (docs): Improvements to retext-spell

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 1, 2020
Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

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

Hi @jazibjafri. Thanks for your PR.

Could you please expand this PR with a bunch of words from that dictionary, rather than just one? For example an acceptable PR would at least go through all plugins on the list and make them consistent, even double checking whether all the plugins in the monorepo are actually in there and making sure they're all spelled consistently.

I mean otherwise I might be tempted to think you're just doing the minimal effort for a certain piece of clothing. And I'm sure you really just wanted to help.

Also, please mark the words you've processed in the linked issue, as requested.

Thanks in advance!

@pvdz pvdz added status: awaiting author response Additional information has been requested from the author type: documentation An issue or pull request for improving or updating Gatsby's documentation and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 1, 2020
@jazibjafri
Copy link
Author

@pvdz hey I understand, I can definitely expand this PR, but don't you think it'll be much work for review if all plugins are processed in one PR? Let me know and I'll update with adding more plugins. Thanks

@pvdz
Copy link
Contributor

pvdz commented Oct 1, 2020

Yes, but so is a single PR for each of those changes. Probably even more so. I'll do the work if you do :)

@jazibjafri
Copy link
Author

@pvdz I'll do it.

@pvdz pvdz marked this pull request as draft October 2, 2020 17:03
@jazibjafri
Copy link
Author

Also, please mark the words you've processed in the linked issue, as requested.

@pvdz Hey, I can't edit the original issue comment to add items to that list. No option to edit is visible to me.

@jazibjafri jazibjafri marked this pull request as ready for review October 2, 2020 18:48
@jazibjafri
Copy link
Author

@pvdz I processed gatsby-transformer-yaml & gatsby-transformer-remark and the PR got lot of changes already. Is it now an acceptable PR? If not, let me know and I'll add some more plugins. Also I cannot edit the original comment, so if you can do that and add the plugins I'm processing here, that'd be great. Thanks in advance.

PS: If I did something wrong in this PR, let me know, and I'll fix it as soon as I can. :)

@jazibjafri jazibjafri requested a review from pvdz October 2, 2020 18:52
@jazibjafri
Copy link
Author

@pvdz Also, tell me what did I do that failed the checks, so I fix them too.

@jazibjafri jazibjafri changed the title fix(docs): dictionary.txt -> fix brand name -> gatsby-transformer-cloudinary fix(docs): fix brand name -> gatsby-transformer-{cloudinary,yaml,remark} Oct 2, 2020
@pvdz
Copy link
Contributor

pvdz commented Nov 9, 2020

I'm not sure how to explain this properly but the bottom line is that I don't think this is a good PR.

If you look through the PR you can see that it leaves certain pages in a worst state, like when you change a header that mentions two package names and you only add backticks to one package name but not the other. I think that inconsistency is objectively a worse state than having no backticks at all.

There are some other cases where I'm not sure the backticks are useful or necessary. Fwiw, I hope you got your shirt, but I am going to close this PR.

@pvdz pvdz closed this Nov 9, 2020
@jazibjafri jazibjafri deleted the fix/dictionary-fix-gatsby-transformer-cloudinary branch November 9, 2020 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting author response Additional information has been requested from the author type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants