-
Notifications
You must be signed in to change notification settings - Fork 77
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
example for adding a new rule #494
base: master
Are you sure you want to change the base?
Conversation
@hippietrail, I've compiled your branch and it looks like it's working just fine. Until now, if more than one rule finds an error That's actually a bug, and I've pushed a commit that should fix it 🤞. Now if you test using |
OK nice. What about something like a Wait a sec... didn't you just say it should show both now? I'll see if I can manage to test via LSP... Nope, sorry. Is that also in the new review docs? I can only see how to test via |
I've set that up. You can either use Are you happy enough with your |
Not yet because though it's working I still don't grok everything. In particular I don't understand the function of all the "that"/"foo" lines I mentioned in #501: I'm looking in the great news docs but I'm not sure if this is missing or I'm just not seeing it. I can see that the By the way, I have more questions in my other PR for a real lint: #502 I'm also stuck on another real lint I've started. I could also do a work-in-progress PR for that one if you like in the hope that me asking all the dumb questions now will make it so future contributors won't have to ask them again (-: |
That's great! But now I can see that with or without |
Honestly, the best way to test suggestions is in an actual editor (like Visual Studio Code). Have you tried running your own build in there? |
|
as per elijah-potter's request
See there for details on how I believe I've followed the instructions but my new rules is ignored and trying it only results in this:
The added rule should look like the that-which rule but silly.