-
Notifications
You must be signed in to change notification settings - Fork 34
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
Move bundle_report rails compatibility logic into a class #137
base: main
Are you sure you want to change the base?
Conversation
61699bf
to
7f7a9df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments
b8e927b
to
74e4bf6
Compare
bb757ad
to
8f93530
Compare
04e5134
to
e8c5911
Compare
@arielj any chances you can review this, please? |
d7e4eb5
to
aa6053d
Compare
I extracted it into a class because I think it will be easier to use the `incompatible_gems_by_state` data there. Based on the code review
80a1716
to
078903a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only have that comment about the test that I think is not testing what it's meant to test, but the rest looks fine to me
@arielj, test updated! could you please approve and merge? |
d1da583
to
550793d
Compare
Description
I extracted it into its own class
Motivation and Context
I think it will be easier to understand the
BundleReport.compatibility
method and its usagealso I will need to use data from the
incompatible_gems_by_state
method in another code.How Has This Been Tested?
There was an existing test case for the
erb_output
method, and I run it locally to make sure it still works.Screenshots:
I will abide by the code of conduct