Skip to content

refactor: automatically generate the rules-list in README #397

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

Merged
merged 3 commits into from
Jul 4, 2021

Conversation

MichaelDeBoey
Copy link
Member

No description provided.

@MichaelDeBoey MichaelDeBoey added the documentation Improvements or additions to documentation label Jun 21, 2021
@MichaelDeBoey MichaelDeBoey requested a review from Belco90 June 21, 2021 10:21
@MichaelDeBoey MichaelDeBoey self-assigned this Jun 21, 2021
Copy link
Member Author

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

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

Seems like some descriptions changed now that we use the rules description directly.

Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

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

Nice PR, it's fantastic to automate this process!

Seems like some descriptions changed now that we use the rules description directly.
I would say that's not a problem. Let's leave the new ones and improve them later if necessary (but looks good to me).

One thing I'd like to request is to check the generated rules list in a test similar to how we check it for recommended presets.

@MichaelDeBoey
Copy link
Member Author

One thing I'd like to request is to check the generated rules list in a test similar to how we check it for recommended presets.

Was thinking about this, but not really sure if we should do so and how would be the best to do this 🤔

@Belco90
Copy link
Member

Belco90 commented Jun 28, 2021

Was thinking about this, but not really sure if we should do so and how would be the best to do this 🤔

It could definitely be tricky with the formatting, but we need some way to automate this to avoid getting the rule list outdated. There are 2 options I can think of for now:

  1. Add a test to check the rule list generated in a specific branch has the same content as the current rule list. Since we have a placeholder in the README to inject the rule list, we can retrieve the current rule list by regexp using those placeholders. We would have to disable the auto-formatting with prettier in that block.
  2. Regenerate the rule list in CI/CD. We would need some workflow to update the rule list if necessary after a change is merged. This could be done by leaving the section empty and injecting the generated rule list in CI/CD in the build step

I prefer option 1 so we have more control about when to generate the rule list, and we also avoid injecting stuff at CI/CD.

@MichaelDeBoey
Copy link
Member Author

Looking at other repos, they don't have anything to test against this.
It's just a thing that needs to be run whenever you create a new rule.
So I wouldn't bother about testing this either tbh 🤷‍♂️

The only thing I would do now is go over all descriptions and see if we want to change them to something more meaningful (like some of them were already in the README list).
This can be done in a separate PR though.

@Belco90
Copy link
Member

Belco90 commented Jun 28, 2021

It's just a thing that needs to be run whenever you create a new rule.

Well, not just when creating a rule, but also when updating or deleting an existing one. Not something critical, but definitely easy to forget about.

Maybe we can add a check to the PR template to indicate "a rule was created/updated/deleted and rule list has been regenerated" or something like that.

@MichaelDeBoey
Copy link
Member Author

Maybe we can add a check to the PR template to indicate "a rule was created/updated/deleted and rule list has been regenerated" or something like that.

Let's create another PR to update our PR template, where we can explain more in depth which things need to be done when creating/updating a rule, ...

@Belco90
Copy link
Member

Belco90 commented Jun 28, 2021

I've created #400 for it.

Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

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

I've pointed some descriptions that I think were better before. Can we update those before going ahead?

@MichaelDeBoey MichaelDeBoey force-pushed the generate-rules-list branch from a162577 to d394609 Compare July 3, 2021 17:21
@MichaelDeBoey MichaelDeBoey requested a review from Belco90 July 3, 2021 17:21
@MichaelDeBoey MichaelDeBoey force-pushed the generate-rules-list branch from d394609 to f922889 Compare July 3, 2021 17:25
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

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

Looks ok now, but it's tricky to review if you force-push after changes requested since I can't compare it against my last review 😞

@Belco90 Belco90 merged commit a6e243a into main Jul 4, 2021
@Belco90 Belco90 deleted the generate-rules-list branch July 4, 2021 12:40
@github-actions
Copy link

github-actions bot commented Jul 6, 2021

🎉 This PR is included in version 4.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants