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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

audit: Port audit_legacy_patches method to rubocop and add tests #2790

Merged
merged 1 commit into from Jun 25, 2017

Conversation

Projects
None yet
3 participants
@GauthamGoli
Copy link
Member

GauthamGoli commented Jun 16, 2017

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

#569
Instead of normalizing patches ( as patches could be in a hash, array or a nested array ) and could be two levels deep with an if guard clause, I'm fetching all the string nodes inside def patches and checking against the url patterns to report rubocop offenses, also went to ~2014 commits to test the cop on legacy patches 馃, but I feel porting this is an overkill as we don't have legacy patches anymore?
patch_problems would be moved to formula_cop.rb once audit_specs porting is finished.

legacy_patches.each { |p| patch_problems(p) }
end

def patch_problems(p)

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Jun 23, 2017

Member

def patch_problems(patch)

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Jun 23, 2017

Looks good so far! Can you give an example of some formulae you tested this on? One tiny nit and then I think good to 馃殺.

@GauthamGoli

This comment has been minimized.

Copy link
Member

GauthamGoli commented Jun 23, 2017

@MikeMcQuaid In homebrew-core I ran a command to find all commits with legacy patches. and then tested it. For example checkout 587658a94b5ee and there is a formula cardpeek with legacy patches.
Edit: More legacy patches. http://paste.ubuntu.com/24933228/

@GauthamGoli GauthamGoli force-pushed the GauthamGoli:audit_legacy_patches_rubocop branch from 8f63781 to 2e82754 Jun 24, 2017

@JCount

JCount approved these changes Jun 24, 2017

@MikeMcQuaid MikeMcQuaid merged commit f4cdd7a into Homebrew:master Jun 25, 2017

3 checks passed

codecov/patch 96.29% of diff hit (target 65.52%)
Details
codecov/project Absolute coverage decreased by -0.06% but relative coverage increased by +30.77% compared to f4f1f1a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Jun 25, 2017

Thanks again @GauthamGoli!

@GauthamGoli

This comment has been minimized.

Copy link
Member

GauthamGoli commented Jun 25, 2017

Thanks for the review @MikeMcQuaid @JCount

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.