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

Made Brakeman CI config stricter #50647

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

igor-alexandrov
Copy link
Contributor

Made Brakeman execution stricter:

  • --ensure-latest ensures that the latest version of Brakeman is used
  • --no-pager is better for CI execution

@rails-bot rails-bot bot added the railties label Jan 8, 2024
@@ -1,4 +1,6 @@
require "rubygems"
require "bundler/setup"

ARGV.unshift("--ensure-latest")
Copy link
Member

Choose a reason for hiding this comment

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

This file is just the generated binstub by bundler, we should not change its content. If this is a better default, we should be wither changing Breakman to always use it.

If we can't, let's pass it in the ci.yml file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafaelfranca I agree with your concerns, however default Rails binstubs are not equal to those which are generated by bundler. Please take a look at file that is generated by bundle binstubs rubocop.

ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)

bundle_binstub = File.expand_path("bundle", __dir__)

if File.file?(bundle_binstub)
  if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
    load(bundle_binstub)
  else
    abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
  end
end

require "rubygems"
require "bundler/setup"

load Gem.bin_path("rubocop", "rubocop")

At the same time default Rails bin/rubocop has the default param which gave me the idea.

require "rubygems"
require "bundler/setup"

# explicit rubocop config increases performance slightly while avoiding config confusion.
ARGV.unshift("--config", File.expand_path("../.rubocop.yml", __dir__))

load Gem.bin_path("rubocop", "rubocop")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that probably we should change both bin/brakeman and force required params in .github/ci.yml.

Copy link
Member

Choose a reason for hiding this comment

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

Nah. I'm fine with this then.

@@ -1,4 +1,6 @@
require "rubygems"
require "bundler/setup"

ARGV.unshift("--ensure-latest")
Copy link
Member

Choose a reason for hiding this comment

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

Nah. I'm fine with this then.

@rafaelfranca rafaelfranca merged commit 5cd66b3 into rails:main Jan 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants