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

Refactor rubocop action #104

Merged
merged 2 commits into from
Feb 7, 2022
Merged

Conversation

davidwessman
Copy link
Contributor

@davidwessman davidwessman commented Jan 31, 2022

No description provided.

@davidwessman davidwessman force-pushed the rubocop-action branch 2 times, most recently from b110f9a to 27912c7 Compare January 31, 2022 19:58
Copy link
Contributor

@andrewshadura andrewshadura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While neither GitHub not GitLab support this natively yet, how about adding .git-blame-ignore-revs file with the 27912c7’s full hash (similarly to e.g. this).

@davidwessman
Copy link
Contributor Author

While neither GitHub not GitLab support this natively yet, how about adding .git-blame-ignore-revs file with the 27912c7’s full hash (similarly to e.g. this).

Good idea 🙂
I added the file, lets hope that Github will support something similar in the future.

@m0n9oose
Copy link
Collaborator

m0n9oose commented Feb 1, 2022

@davidwessman can you explain why exactly Standard is any better than good ol' Rubocop? I agree, we need to update it and maybe adopt some changes in rules, but I don't see why we need to replace it.

I was thinking about using the Rails approach https://github.com/rails/rails/blob/4a78dcb326e57b5035c4df322e31875bfa74ae23/.github/workflows/rubocop.yml

@davidwessman
Copy link
Contributor Author

I started with doing that, but then I ran into the outdated rubocop config, and did not know how to update the rules.

The main reason to use standard is to avoid having to device on all the new rules and get a nice default.

I can fix an updated action using rubocop directly without changing the .rubocop.yml

@davidwessman
Copy link
Contributor Author

@m0n9oose
Copy link
Collaborator

m0n9oose commented Feb 1, 2022

@davidwessman i don't see the point of this tool. Rubocop has default settings as well. You don't have to configure every single cop. Also, as far as I see, this is just a wrapper for Rubocop.

I don't mean to look into new tools, but this one seems just a wrapper with minimal value. Not to mention we'll have to update the whole codebase to adopt the new code style.

Let's stick with Rubocop and just update it.

@davidwessman davidwessman force-pushed the rubocop-action branch 6 times, most recently from 45216b5 to d7eac46 Compare February 2, 2022 06:00
@davidwessman
Copy link
Contributor Author

@m0n9oose Now it uses rubocop with the same config all the way 🙂

@davidwessman davidwessman changed the title Replaces rubocop with standard Refactor rubocop action Feb 2, 2022
@stanhu stanhu merged commit 22cc1b0 into omniauth:master Feb 7, 2022
@davidwessman davidwessman deleted the rubocop-action branch February 7, 2022 07:26
@stanhu stanhu mentioned this pull request Feb 8, 2022
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.

5 participants