Skip to content

Conversation

@lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Jun 10, 2025

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-h1 rule, which was newly added in #411.

I've updated the regex pattern to correctly handle the JSON front matter title and 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.)

@lumirlumir lumirlumir changed the title feat: update no-multiple-h1 to recognize JSON frontmatter. feat: update no-multiple-h1 rule to recognize JSON frontmatter. Jun 10, 2025
@Pixel998
Copy link
Contributor

Pixel998 commented Jun 10, 2025

@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.

@nzakas
Copy link
Member

nzakas commented Jun 10, 2025

@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.

@Pixel998
Copy link
Contributor

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.

@nzakas
Copy link
Member

nzakas commented Jun 10, 2025

@Pixel998 While I can appreciate what you're saying, we're going to stick with this PR. Thanks for your understanding.

@lumirlumir
Copy link
Member Author

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.

@lumirlumir lumirlumir changed the title feat: update no-multiple-h1 rule to recognize JSON frontmatter. feat: update no-multiple-h1 rule to recognize JSON frontmatter Jun 12, 2025
@lumirlumir lumirlumir marked this pull request as ready for review June 13, 2025 08:37
@lumirlumir lumirlumir marked this pull request as draft June 13, 2025 08:38
Comment on lines +69 to +71
---
[ "title" ]
---
Copy link
Member Author

Choose a reason for hiding this comment

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

image

The formatting of the code is consistent in the file view, but the GitHub diff displays it somewhat incorrectly.

@lumirlumir lumirlumir marked this pull request as ready for review June 13, 2025 08:50
@lumirlumir lumirlumir requested a review from a team June 13, 2025 08:50
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jun 13, 2025
Copy link
Member

@nzakas nzakas 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! Would like another review before merging.

@nzakas nzakas moved this from Needs Triage to Second Review Needed in Triage Jun 16, 2025
@lumirlumir lumirlumir requested a review from a team June 17, 2025 08:59
@Pixel998
Copy link
Contributor

LGTM, thanks!

@nzakas nzakas merged commit 33dda18 into main Jun 17, 2025
23 checks passed
@nzakas nzakas deleted the feat-update-no-multiple-h1-to-recognize-json-frontmatter branch June 17, 2025 14:29
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

4 participants