Skip to content
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

Full-file version of new-from-rev? #2261

Closed
natefinch opened this issue Sep 27, 2021 · 5 comments
Closed

Full-file version of new-from-rev? #2261

natefinch opened this issue Sep 27, 2021 · 5 comments
Labels
enhancement New feature or improvement

Comments

@natefinch
Copy link
Contributor

natefinch commented Sep 27, 2021

Your feature request related to a problem? Please describe.

I want to encourage people to fix linting errors in their repos without requiring them to fix every single error in the whole repo at once. New-From-rev seems like it would be good for this, except that it's too narrowly focused. It forces you to fix linting errors on any lines you change or new code you add... but if you never touch other lines of code, it'll never bug you about fixing them.

I'd like a version of new-from-rev where it'll show all linting errors from any file you touch. That way the ripple effects go a lot further, and you'll be a lot more likely to fix more things as you go along.

Describe the solution you'd like.

I figure this could be an almost copy and paste of the new-from-rev code, except any matches in files that have been touched are shown.

Describe alternatives you've considered.

You could have a script that makes a fake patch that includes the full code of all the lines in the PR and use new-from-patch, but that seems like a huge hack that might be prone to breaking.

Additional context.

I am willing to code this up if people think it's a good idea.

@natefinch natefinch added the enhancement New feature or improvement label Sep 27, 2021
@SVilgelm
Copy link
Member

@natefinch I'm ok with this feature, it has pros and cons. Please feel free to implement this. But think about not copy-pasting, but refactoring. I don't want to see one more flag with similar functionality, it would be better if we have one more flag that will control the functionality of new-from-rev

@natefinch
Copy link
Contributor Author

Yep. That's exactly how I'm doing it. Just a bool that applies to either new from rev or new from patch. I should have a PR ready soon. Writing tests now

@natefinch
Copy link
Contributor Author

I thought about a different way this could be handled, but I think it has an annoying and fatal flaw.

There could be a "distance from diff" value that defaults to 0, i.e. issues only in exact lines you touch. If you set it to -1, it would do the whole file, as above. Otherwise you could set it to like 10, and any issues within 10 lines of lines you've changed would "count". That sounds cool, except every time you fix a line 10 lines up, the demarcation moves up 10 lines from that change, potentially finding more issues, etc. etc. Super annoying. At least with full file, you know when you're done, and there's no chance a fix to an old error brings in a new one.

@bombsimon
Copy link
Member

Sounds like a good start to just mark the whole file, I think the use cases for the distance feature would be very few and not worth a complex implementation.

@ldez
Copy link
Member

ldez commented Oct 1, 2021

Closed by #2264

@ldez ldez closed this as completed Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement
Projects
None yet
Development

No branches or pull requests

4 participants