-
Notifications
You must be signed in to change notification settings - Fork 80
feat: update no-multiple-h1 rule to recognize JSON frontmatter
#413
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: update no-multiple-h1 rule to recognize JSON frontmatter
#413
Conversation
no-multiple-h1 to recognize JSON frontmatter.no-multiple-h1 rule to recognize JSON frontmatter.
|
@lumirlumir I implemented this change locally. I am just waiting for the other PR to merge first. Edit: I will submit it and mark it as draft. |
|
@Pixel998 We generally go with the person who first submitted a PR. With so many contributors, it's difficult for us to know who is working on what unless an issue or a PR is submitted on GitHub. The key to maintaining a popular project like ESLint is clear communication and in this case it doesn't look like there was any communication about doing this work. So I'd like to go with @lumirlumir's PR for this feature and I'd invite you to help review it because you're familiar with the change. Going forward, I'd like to ask you both to please open issues before you start work on a feature. That way, you're communicating your intent to work on something and we can avoid having two people working on the same thing in the future. |
|
I understand the situation and sincerely apologize for the miscommunication on my part regarding this feature. I completely respect the policy of prioritizing the person who first submits a PR, as clear communication is crucial for maintaining a project like ESLint. My intention was not to circumvent the process. I had already implemented the changes for the no-multiple-h1 rule to recognize JSON frontmatter, and my work is complete. The reason for the delay in submitting my pull request was to wait for the JSON frontmatter support to be merged first, as it was a prerequisite for my changes. Given that my work is ready and lumirlumir hasn't started work on this, I would respectfully ask if we could consider an exception in this specific instance. Moving forward, I'm fully committed to opening issues before starting any new work. |
|
@Pixel998 While I can appreciate what you're saying, we're going to stick with this PR. Thanks for your understanding. |
|
Thank you all for clarifying and understanding the situation. I truly realized how important it is to raise an issue and have a conversation before proceeding with specific features. I'll make sure to avoid causing confusion next time. |
…pdate-no-multiple-h1-to-recognize-json-frontmatter
no-multiple-h1 rule to recognize JSON frontmatter.no-multiple-h1 rule to recognize JSON frontmatter
| --- | ||
| [ "title" ] | ||
| --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Would like another review before merging.
|
LGTM, thanks! |

Prerequisites checklist
What is the purpose of this pull request?
In this PR, I've added support for the JSON front matter feature to the
no-multiple-h1rule, which was newly added in #411.I've updated the regex pattern to correctly handle the JSON front matter
titleand some edge cases.What changes did you make? (Give an overview)
Related Issues
Prerequisite: #411
Is there anything you'd like reviewers to focus on?
I tried to update the regex as accurately as possible, but there might be some edge cases I’ve missed.
It would be nice if someone could find more—excluding those that cause parsing errors. (I didn’t include cases that cause parsing errors.)