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

chore: add missing ':'s in registry cSpell ignore lists #3430

Closed
wants to merge 1 commit into from

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Oct 24, 2023

Not mandatory, but this creates consistency with other cSpell:ignore: lists we have

@svrnm svrnm requested a review from a team October 24, 2023 07:51
Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

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

If we haven't yet, could we document this in a contribution guide together with other particularities of our docs? If that doc doesn't exist, I can create an issue and work on it.

@svrnm
Copy link
Member Author

svrnm commented Oct 24, 2023

If we haven't yet, could we document this in a contribution guide together with other particularities of our docs? If that doc doesn't exist, I can create an issue and work on it.

our contribution guidelines are outdated, we had an issue for that in the past (#897, #1890), so yes, all help is appreciated here!

@simi
Copy link
Contributor

simi commented Oct 30, 2023

I have not seen ignore: in docs, shouldn't it be opposite? Remove : in cSpell ignore lists? Or is there any reason to keep those in there?

https://cspell.org/configuration/document-settings/

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Catching up on PRs. @simi is right, we don't need a :, ... unless we're encoding the ignore rule as part of the Hugo front-matter. In such cases, we need the colon because cSpell:ignore is being used as a YAML key name.

I'd suggest not making these changes.

@chalin
Copy link
Contributor

chalin commented Oct 31, 2023

With @cartermp's thumbs up, I'm going to close this for now. @svrnm - feel free to reopen if you'd like to explore this further.

@chalin chalin closed this Oct 31, 2023
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.

5 participants