-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add support for metadata to all rules #5417
Comments
Per the Gitter chat, I think @nzakas' vision was to do documentation first, then start converting rules accordingly. Should this issue be retitled and/or split accordingly? |
Should we add a boolean property indicating whether the rule is fixable? It would help with the approach for autofixing mentioned in #5329. We'll also need it to autogenerate the docs. |
@alberto There's a |
I started re-writing documentation for this today, but I'm not sure if I should replace all the old syntax with the new one, in documentation, or should I add new one, and mark old as deprecated? Since both of them supported, it feels like latter is probably more correct, but it also feels like it might confuse users. |
I'd say create separate docs alongside the existing ones, but don't link to them. Instead, we'll use the new docs to get people started on updating the core rules. Once the core rules are updated and we've validated that the new docs are good enough for that purpose, we can link to the new rules documentation and mark the old ones as deprecated. |
Sounds good to me. So I'll clone "Working with Rules" documentation page and change it for the new rule structure. Should be able to take care of that this week. |
Hi @ilyavolodin, I think this is a great idea! From what I understand, the final goal is to autogenerate the list of rules in the README.md file, right? I would love to help out, would there be anything you need some assistance with? :) |
@vitorbal The goal is to auto-generate the list of rules (at minimum), but also look into auto-generating part of rule documentation, as well as provide structure for some of the internal information. Maybe even do something about rules for plugins at some point. |
@ilyavolodin That sounds great! Yes, I would love to contribute :) How should we coordinate it? Should we share a branch? |
@vitorbal I don't think we need a separate branch. Those changes are non-breaking (I already modified the core to support both old and new style). If you want to, you can start from the bottom of the rules in the |
@ilyavolodin As discussed, I created a jscodeshift transform to automate this. I ran it locally and it works like a charm. I'll send a pull request once #5719 gets merged! |
If like to suggest that the transforms be done in batches rather than all at once to avoid merge conflicts with other PRs. |
@vitorbal Concerning the excellent goal to auto-generate the rule index as you mentioned in #5417 (comment) just wanted you to know:
|
@pedrottimark No metadata for removed rules, that would have to be a separate file somewhere. Index would be generated as part of the release script, so we will be able to add that file in as well. Format would be a simple change in the template. Rule description - yes, that we would have to change once you guys agree on the format, but again, not any more complicated then what it already is (although much bigger chance for merge conflicts:-) |
In case I was unclear, am suggesting that the improved and ready-to-be-merged rule descriptions in README.md would be the source for meta.docs.description in rule files whether by editing or code transform. The rule files become the source after the index file can be auto-generated. We can update the h1 headings in rule docs according to README.md (or rule files) in batches as part of the task to edit example sentences and option descriptions. By the way, I used schema to develop initial guidelines for option descriptions. With a consistent pattern, there is potential for automated checking that rule docs include info about all of the options. Can you explain “bigger chance for merge conflicts” so I do not cause them by lack of understanding? |
@pedrottimark Ah ok, got it. So you want to merge #5778 first, and then generate metadata off of the new version, right? I know that @vitorbal had to make some manual changes (specifically around Bigger chance for merge conflicts is due to the fact that descriptions will now live inside the rules themselves. Rules change much more often then Index.md does, hence bigger chance. |
@pedrottimark @ilyavolodin hi guys, thanks for all the feedback. @ilyavolodin is right, I can easily re-generate the metadata i need for the transform, as I wrote a small script that creates a "mapping" for them by scraping the eslint.org/docs/rules page. So I would propose that we merge #5778 first and I will re-run the scraper and transform after that. I can also do the changes in batches as @nzakas suggested. Would 40 rules per PR be okay? That would be roughly 5 PR's in total. @pedrottimark concerning the meta-data for removed rules, I have access to those too since the scraper also picks them up. But since the transform doesn't know where to map them to, they won't be part of the PR. What's the plan for those, do we want to store them somewhere separately? I can auto-generate a file for them too if that's wanted. |
@ilyavolodin Yes, #5778 is ready, I think. @vitorbal Great work, and glad you are enjoying it! Here is content that seems to come from another source than rule meta properties:
Without limiting your creativity in how to generate the rule index file, the arbitrary order makes me think of a template file like current README.md without lists/tables. Whether you could or should build tables for each section in Jekyll with Liquid templating language goes beyond my knowledge. What about cn.eslint.org? Would its rule index file be edited as static text as en has been so far? |
@pedrottimark Thanks, I am definitely enjoying it :) My experience with Jekyll and Liquid templating is also very limited, but I imagine if we decide on the table output we could have a partial that generates a table using the tablerow tag, and feed it with a JSON data file with the category -> rules mapping that could be auto-generated from the meta properties during the build process. But of course, I would not like to step on @ilyavolodin's toes as this is his baby :) @ilyavolodin is this somewhat similar to what you had in mind? |
Docs: Add a new page for working with rules (refs #5417)
Add metadata to the existing rules. Batch 2.
Update: Add metadata to existing rules - Batch 3 (refs #5417)
Add metadata to the existing rules. Batch 4.
Chore: Add metadata to existing rules - Batch 4 (refs #5417)
Chore: Add metadata to existing rules - Batch 5 (refs eslint#5417)
Chore: Add metadata to existing rules - Batch 6 of 7 (refs eslint#5417)
Chore: Add metadata to existing rules - Batch 7 of 7 (refs eslint#5417)
@nzakas @ilyavolodin |
Let's wait and do that on Tuesday (we have a release tomorrow and possible patch release Monday) so we can plan how to announce the change with the next release. |
@nzakas @ilyavolodin @pedrottimark Would love to help out and get the ball rolling for this one. How do we usually go about marking an old doc file as deprecated? Do we keep it and rename it, and link to it from the new doc that super-seeded it? |
@vitorbal Are you talking about |
It looked to me like all the existing links to working-with-rules are better pointing to the new content, and also links on search engines. Suggest:
|
Instead of |
what I had in mind:
|
I'm finishing this up. Updating docs now. |
I've added support for new rule format and metadata to two existing rules accessor-pairs and array-bracket-spacing in v2.0.0 release.
This new format should be documented and the rest of the rules should be converted to use it. I'm not sure what would be the best way to do that, in stages or as a bulk change.
The text was updated successfully, but these errors were encountered: