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.

Sign up for GitHub

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

Jump to bottom

Blast out changes to all repos for the codeclimate channel #44

Closed
77 tasks done
Fryguy opened this issue Mar 1, 2024 · 5 comments
Closed
77 tasks done

Blast out changes to all repos for the codeclimate channel #44

Fryguy opened this issue Mar 1, 2024 · 5 comments
Labels
test

Comments

@Fryguy
Copy link
Member

Fryguy commented Mar 1, 2024 •
edited by kbrock
Loading

@Fryguy Fryguy added the enhancement New feature or request label Mar 1, 2024
@Fryguy
Copy link
Member Author

Fryguy commented Mar 1, 2024

cc @agrare @jrafanie @bdunne @kbrock

This may cause repos to go "red" for CodeClimate because it will actually find newer things that are in the newer codeclimate.

I wish there was a way to have central configuration for this particular thing, but I don't think codeclimate supports that :(

@Fryguy
Copy link
Member Author

Fryguy commented Mar 1, 2024 •
edited
Loading

@agrare @jrafanie @bdunne @kbrock I am seeing some common codeclimate patterns (example) that I think we can fix with a few simple decisions. Please let me know your thoughts and then I can make the necessary changes.

  1. https://docs.rubocop.org/rubocop/cops_gemspec.html#gemspecdevelopmentdependencies

    This defaults to "Gemfile", meaning they don't want to see development dependencies in the gemspec. Personally, I think that's weird, and I see nothing wrong with .gemspec. In fact, when it's in gemspec you can then also see it in rubygems.org. I can't find anything in the Ruby style guide about this decision. Discussion about it was here (https://github.com/rubocop/rubocop/pull/11469), and the choice of default was rather arbitrary, with quite a few against.

    I'm leaning towards changing the default for this to .gemspec or even disabling it altogether

  2. https://docs.rubocop.org/rubocop/cops_gemspec.html#gemspecrequiremfa

    I'm ok with this one, and it would be trivial to blast this out to all of our gems. If you agree I'll do it.

  3. https://docs.rubocop.org/rubocop/cops_gemspec.html#gemspecrequiredrubyversion

    While I would like to keep this one and think we should set it everywhere, the way they designed this is that the TargetRubyVersion must match the minimum specified in the gemspec, so since we set our TargetRubyVersion to 3.0, we'd be required to do required_ruby_version >= 3.0, and I don't like that

    I'm leaning towards disabling this cop.

@kbrock
Copy link
Member

kbrock commented Mar 1, 2024

  1. I prefer dev gems in the gemspec (except for byebug) - Gemfile should be used for pegging a dev environment and not for dependencies. default to gemspec (Sometimes you need to peg the rails version, so maybe disabling is the way to go)
  2. I tended not to go this route, but not my hill. blast to yes
  3. Yes, I like to set ruby version in every gem. Agree we want to set our own preferences though. disable

@jrafanie
Copy link
Member

jrafanie commented Mar 1, 2024 •
edited
Loading

  1. I also think gemspec is fine. I forgot but apparently, you can also install a gems dev dependencies: gem install --dev your_gem. That goes along with the niceties of showing people in rubyems.org. Honestly, I think Gemfile is nice for stuff you're hacking on or debugging but once you want to publish the gem, any dev dependencies you actively use should probably be promoted to the gemspec as such. Either way, I don't think this is something we really care about flagging.
  2. Sure. Not a big deal but we should do that. Blast away.
  3. This might be branch specific. I'm fine with enforcing the minor ruby version on release branches but on master, we should ignore this.

@Fryguy Fryguy mentioned this issue Mar 1, 2024
Merged
@kbrock kbrock added test and removed enhancement New feature or request labels Mar 6, 2024
@kbrock
Copy link
Member

kbrock commented Mar 6, 2024

all issues have been merged.

@kbrock kbrock closed this as completed Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kbrock @jrafanie @Fryguy