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

Switch to --auto-correct-all #127

Closed
wants to merge 1 commit into from
Closed

Conversation

AlecRust
Copy link

@AlecRust AlecRust commented Sep 21, 2020

Please see #123, #101.

@mgonc
Copy link

mgonc commented Sep 25, 2020

@ngouy
Copy link
Contributor

ngouy commented Oct 5, 2020

With newer version of rubocop (0.92) and fix about the json output (see #129), I don't have any issue (all my files have a frozen_string_literal: true and auto correct is working).
Why must we run also unsafe corrections? I feel like auto correct unsafe corrections is, well, not safe, its more of an improvement.

Why is it a requirement?

Maybe we can try to add on option in the extension settings to toggle this behaviour?

@AlecRust
Copy link
Author

AlecRust commented Oct 5, 2020

@ngouy I agree it would be better to add an option for this. This PR fixes the behaviour to before the Rubocop change, but perhaps no longer necessary.

I don't have any issue (all my files have a frozen_string_literal: true and auto correct is working).

Strange, my understanding was this cop was unsafe in terms of autocorrect (rubocop/rubocop#8529).

@ngouy
Copy link
Contributor

ngouy commented Oct 5, 2020

Maybe I didn't quite understand the issue with the frize_string_litteral thing. Is the issue that you expect it to be autocorrected when you don't have it in your file? Or is the issue that when you have it the autocorrect doesn't works for other policies?

Or asked otherwise : What is broken that this PR fix. Is that "just" that autoccorect only correct safe policies now or is there a real underlying bug? (thought it was to fix this : #123 (comment))

As you said yourself in #101 maybe the real "fix" (if there is no bug) may be to add an option.

@AlecRust
Copy link
Author

AlecRust commented Oct 5, 2020

Is the issue that you expect it to be autocorrected when you don't have it in your file?

Yes. This was the behaviour I experienced before this regression (caused by new Rubocop release).

@ngouy
Copy link
Contributor

ngouy commented Oct 5, 2020

It's a kinda messy in last issues, some duplicates and sometimes many different bugs in a single one issue ^^' Hard to understand it all

@misogi
Copy link
Owner

misogi commented Nov 7, 2020

Could you add an option (ex: safeAutoCorrect) that toggles --auto-correct or --auto-correct-all?

https://docs.rubocop.org/rubocop/usage/auto_correct.html

@misogi misogi closed this Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants