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

Register policies directories for Rails 8 code statistics #833

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

javierav
Copy link

@javierav javierav commented Oct 1, 2024

This PR add a Rails::Railtie to Pundit for configure the new stats directories in upcomming Rails 8.

@Burgestrand
Copy link
Member

Thanks, this is neat! I need to find the time to test this out.

@Burgestrand Burgestrand added the simmering undecided but generally optimistic label Oct 8, 2024
@Burgestrand Burgestrand self-requested a review October 8, 2024 08:04
@Burgestrand Burgestrand self-assigned this Oct 8, 2024
lib/pundit/railtie.rb Outdated Show resolved Hide resolved
@javierav
Copy link
Author

javierav commented Oct 9, 2024

@Burgestrand Can it be interesting to support older versions of rails? In such cases you would have to use a rake task (I never understood this ugly part of rails)

if Rails.version.to_f < 8.0
  rake_tasks do
    load "pundit/rails/tasks/pundit.rake"
  end
else
  initializer "pundit.stats_directories" do
    require "rails/code_statistics"

    if Rails.root.join("app/policies").directory?
      Rails::CodeStatistics.register_directory("Policies", "app/policies")
    end

    if Rails.root.join("test/policies").directory?
      Rails::CodeStatistics.register_directory("Policy tests", "test/policies", test_directory: true)
    end
  end
end
# pundit/rails/tasks/pundit.rake

task stats: "pundit:stats"

namespace :pundit do
  task :stats do
    require "rails/code_statistics"

    if Rails.root.join("app/policies").directory?
      ::STATS_DIRECTORIES << ["Policies",  "app/policies"]
    end

    if Rails.root.join("test/policies").directory?
      ::STATS_DIRECTORIES << ["Policy tests", "test/policies"]
      CodeStatistics::TEST_TYPES << "Policy tests"
    end
  end
end

I have also removed the RSpec part since rspec-rails gem is responsible for adding the tests to the statistics here

@Burgestrand
Copy link
Member

@Burgestrand Can it be interesting to support older versions of rails? In such cases you would have to use a rake task (I never understood this ugly part of rails)

Gut feeling is "not really". I don't know how much you use rails stats, but for me it's probably less than once a year and it's not exactly an often-requested feature of Pundit 😄

This PR change is small enough and uses documented APIs, which makes me comfortable merging it. Whatever we merge we'll have to support for a relatively long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
simmering undecided but generally optimistic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants