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

fix: Support older version of Pundit #686

Merged
merged 1 commit into from Feb 20, 2022

Conversation

rickychilcott
Copy link
Contributor

@rickychilcott rickychilcott commented Feb 19, 2022

Description

Pundit 2.2.0 changes the Module that is required to be included, which is a breaking change. Since Pundit 2.1.x is still very viable for most apps, support both options.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Pundit 2.2.0 changes the Module that is required to be included, which is a breaking change. Since Pundit 2.1.x is still very viable for most apps, support both options.
@codeclimate
Copy link

codeclimate bot commented Feb 19, 2022

Code Climate has analyzed commit 6a536f6 and detected 0 issues on this pull request.

View more on Code Climate.

@rickychilcott
Copy link
Contributor Author

Not sure what you think of this one @adrianthedev.

I'm surprised that varvet/pundit@4d9b584 wasn't considered a breaking change (i.e. bumping to Pundit 3.0), but either way, I can't currently fix all of my OTHER dependencies to jump to Pundit v2.2 yet.

Does this simple version work or would you rather a Pundit version check?

@codecov
Copy link

codecov bot commented Feb 19, 2022

Codecov Report

Merging #686 (6a536f6) into main (030f2fc) will decrease coverage by 0.08%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #686      +/-   ##
==========================================
- Coverage   93.77%   93.69%   -0.09%     
==========================================
  Files         364      364              
  Lines        6536     6548      +12     
==========================================
+ Hits         6129     6135       +6     
- Misses        407      413       +6     
Impacted Files Coverage Δ
app/controllers/avo/application_controller.rb 85.71% <66.66%> (-0.12%) ⬇️
lib/avo/fields/key_value_field.rb 79.54% <0.00%> (-9.35%) ⬇️
app/controllers/avo/base_controller.rb 99.39% <0.00%> (-0.01%) ⬇️
spec/system/avo/default_field_spec.rb 95.12% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 030f2fc...6a536f6. Read the comment docs.

@rickychilcott rickychilcott changed the title Support older version of Pundit fix: Support older version of Pundit Feb 19, 2022
@adrianthedev
Copy link
Collaborator

Hey @rickychilcott. Yeah, this might work. It seems I haven't thought about this change as being a breaking change, but what you're saying makes sense.

I'll check it out and merge this before Monday's release.

@adrianthedev adrianthedev merged commit d061e58 into avo-hq:main Feb 20, 2022
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