-
-
Notifications
You must be signed in to change notification settings - Fork 243
Add warning for suggested changes to rule docs #899
Comments
Hi, I am new to open source and I would like to work on this issue. Please guide me further on what needs to be done. |
@pranay101 thanks for the interest! We would need to add a link here: And then add a page that it links to describing what suggestions are and how they might produce changes that alter the code behavior. |
hey @nzakas which link would you like me to redirect the users to? |
We can probably start with /user-guide/fixes-and-suggestions. Note that we would also need to add this into the main eslint repo in the docs folder (which is the source of truth for the website) |
Hello! Is this issue already being worked on, please? If not, then I would like to take it. Thanks! |
@AkashaRojee you are free to work on it 👍 |
Created /fixes-and-suggestions/no-unsafe-negation.md under /user-guide, and added fixes and suggestions using information from the rule's test source. This file is a first draft to verify the content. Once approved, it will be added to the main eslint repo, and future PRs for this file will be accepted only from there. There are also parts that could be removed, and instead generated through the main eslint repo's Makefile. Some of these parts are: - front matter - comment for non-native file - Version section - Resources section Refs eslint#899
Great! I worked on a first draft of the fixes and suggestions for Please, find it here. I have a few questions about the next steps. Once they are confirmed, I will proceed to the fixes and suggestions for the other rules. 1. Is the content in the first draft correct? I used the information from the rule's test source. Is there anything I should add/change? 2. Which folder/file structure is preferred? Option 1: Option 2: Depending on the above, I will then add the correct 3. How to proceed with the PRs for the website repo and the main repo? I understand that we would also need to add this content into the main repo in While inspecting the main repo, I saw that a Makefile is used to automate the generation of parts of the documentation, e.g.:
Should I remove these sections from the file, and modify the Makefile accordingly? Thanks! |
Thanks so much for working on this. I think we may have a misunderstanding here: we aren’t looking for individual updates to rules where we list what happens with each suggestion. All we are doing here is adding a link from the rules index page that explains suggestions in all rules may not be safe. As such, option 1 is the best choice to display a generic message about why suggestions may not be safe. Regarding where to make changes: the content of the warning should be in the eslint repo. There is no need to make changes to the Makefile. You can then add a link to the rules index in the website repo. We will wait to merge that until the next release, when the new content from the eslint repo is copied over. |
@nzakas Thanks for the clarification. Sorry for the delay, I was out of action. I'll get to it this week 🚀 |
No problem, thanks for the followup. |
In the 2021-12-16 TSC meeting, we discussed feat: Fixer for missing unicode flag in no-misleading-character-class. That PR adds an editor suggestion that will add the
u
flag. Sometimes, adding that flag may change behavior of other portions of the regular expression unrelated to the misleading character class. While adding theu
flag is probably the best fix most of the time, we're implementing the change as a suggestion rather than an autofix because we can't be sure the indirect behavior changes are correct.More generally, we want to make sure users know that they should consider all possible implications of the behavior change before applying editor suggestions. Currently, documentation pages for rules with suggestions (like
no-unsafe-negation
) include a light bulb icon and the text "Some problems reported by this rule are manually fixable by editor suggestions."In the meeting, we agreed to change the wording to "Some problems reported by this rule
are manuallymay be fixable by editor suggestions. (Warning)" and link "Warning" to a page that describes the potential pitfalls.The text was updated successfully, but these errors were encountered: