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

Improve the purpose/usability of rubocop-github #109

Open
4 of 5 tasks
bensheldon opened this issue Aug 3, 2022 · 0 comments
Open
4 of 5 tasks

Improve the purpose/usability of rubocop-github #109

bensheldon opened this issue Aug 3, 2022 · 0 comments

Comments

@bensheldon
Copy link
Contributor

bensheldon commented Aug 3, 2022

The current purpose of github-rubocop

The current purpose of this Repository from the README:

This repository provides recommended RuboCop configuration and additional Cops for use on GitHub open source and internal Ruby projects.

What does that include?

I think that includes:

  • A. Baseline rules (config/default.yml) for upstream rubocop, rubocop-rails, rubocop-performance, etc. that is a consistent starting place for GitHub-maintained projects. Individual projects/teams are empowered to customize them to their preference.
  • B. GitHub-specific cops that are specific to "GitHub open source and internal Ruby projects". For example:
    • Cops that correspond to specific internal APIs, or Github-specific monkeypatches/behavior on specific projects.
    • Cops that are relevant to all GitHub-managed projects (but not any project anywhere)

What doesn't that include?

I think that does not include:

  • C. Cops that would be applicable to all projects within and outside of GitHub.

Why distinguish GitHub from Everyone?

I think we will benefit the broader community/ecosystem if we make Cops that are applicable to all projects available without having to pull in Rules and Cops that are only narrowly applicable to us.

I think we also will benefit ourselves by sending a stronger signal that we believe those Cops are good for everyone and invite community discussion and participation.

Where will the Cops go instead?

I think this workflow matches how other upstream contributions are managed internally at GitHub:

  1. Propose the change upstream in the core open source project, where they have maximal visibility and discussion/feedback.
  2. Patch them provisionally into GitHub's project(s), for immediate benefit. That could include this repository, or the proposer's Ruby project.
  3. ...if the proposal is accepted, remove our patch and pull the updated upstream
  4. ...if the proposal is not accepted, we usually learn something about our proposal, and we then choose whether to maintain the patch ourselves, discard it, or identify a better open-source home for it.

What is the current disposition of Cops?

I definitely think this is more a molehill than a a mountain. And I think making the disposition of cops exemplary will help model the outcome we want to see (👪=Everyone, 🐙=GitHub, 🚝=GitHub's Monolith, ❌=redundant/removable)

  • 🐙 insecure_hash_algorithm.rb: 🐙 All-GitHub. FIPS compliance
  • 👪 / ❌ rails_application_record.rb: Everyone (duplicate in rubocop-rails)
  • 🚝 rails_controller_render_action_symbol.rb: GitHub Monolith (performance-related)
  • 🚝 rails_controller_render_literal.rb GitHub Monolith (performance-related)
  • 🚝 / 👪 rails_controller_render_paths_exist.rb: GitHub Monolith (performance-related) or Rubocop-Rails?
  • 👪 rails_controller_render_shorthand.rb: Everyone (I think I’ve seen this in the community, but now can’t find it)
  • 👪 / ❌ rails_render_inline.rb: Everyone (duplicate in rubocop-rails)
  • 🚝 rails_render_object_collection.rb: GitHub Monolith?
  • 🚝 rails_view_render_literal.rb: GitHub Monolith?
  • 🚝 / 👪 rails_view_render_paths_exist.rb: GitHub Monolith? Or Rubocop-Rails.
  • 🚝 rails_view_render_shorthand.rb: GitHub Monolith (performance-related)

That adds up to:

  • 🐙 1 cop that’s relevant to all GitHub projects. Because of our Security requirements (though reasonably applicable to everyone)
  • 🚝 5 cops that are only relevant to the Monolith. Mainly because of performance patches we’ve done to render that I don’t think are widely relevant (e.g. Rails flexible development vs GitHub disciplined performance)
  • 🚝 / 👪: 2 cops I’m not sure if they’re relevant to everyone, I’m leaning towards Monolith.
  • 👪: 6 cops that are relevant to everyone, mainly Accessibility
  • ❌: 2 cops that have been upstreamed that we can remove

The Proposal

@bensheldon bensheldon changed the title Discussion: Disposition of this repository Improve the usability of rubocop-github Oct 11, 2022
@bensheldon bensheldon changed the title Improve the usability of rubocop-github Improve the purpose/usability of rubocop-github Oct 11, 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

No branches or pull requests

1 participant