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

lines_cop: add ENV.universal_binary audit exemption for wine #3291

Merged

Conversation

JCount
Copy link
Contributor

@JCount JCount commented Oct 9, 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?

add ENV.universal_binary audit exception for wine and corresponding test coverage

@JCount
Copy link
Contributor Author

JCount commented Oct 9, 2017

cc @ilovezfs

@@ -419,7 +419,22 @@ class Foo < Formula
end
end

it "with ENV.universal_binary" do
it "with ENV.universal_binary exempted formula" do
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this into a proper sentence? Thanks. 😉

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 was only matching the one above it. So, inconsistency or proper sentences?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally proper sentences for all, or specify instead of it.

Copy link
Contributor Author

@JCount JCount Oct 9, 2017

Choose a reason for hiding this comment

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

The full fragment is: 'When auditing formula with ENV.universal_binary exempted formula' any suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JCount
Copy link
Contributor Author

JCount commented Oct 9, 2017

@reitermarkus Is something like this what you had in mind?

@JCount JCount force-pushed the lines-cop-fix-wine-universal-binary branch 2 times, most recently from 7aa72ef to e61114e Compare October 9, 2017 18:27
@JCount
Copy link
Contributor Author

JCount commented Oct 13, 2017

@reitermarkus gentle ping on this

@reitermarkus
Copy link
Member

Something like this is what I had in mind.

@JCount
Copy link
Contributor Author

JCount commented Oct 13, 2017

Since this has the context When auditing formulae prepended to both tests, making the wording following the it into complete sentences, would, in my mind, only serve to make things less intelligible.

@ilovezfs
Copy link
Contributor

ilovezfs commented Oct 14, 2017

it "is as quiet as a church mouse when there is an exemption from the deprecation of ENV.universal_binary" do

@reitermarkus
Copy link
Member

… in my mind, only serve to make things less intelligible.

It should be possible to make it more intelligible than “when auditing formulae it with a build.universal? exemption”.

Also, the context is pretty redundant, so this could actually be removed, and we'd just have it "does not report build.universal? offenses for Wine" do, which would be pretty clear to me.

@JCount JCount changed the title lines_cop: add ENV.universal_binary audit exception for wine lines_cop: add ENV.universal_binary audit exemption for wine Oct 15, 2017
@JCount JCount force-pushed the lines-cop-fix-wine-universal-binary branch from e61114e to 84f2db8 Compare October 15, 2017 14:27
@JCount JCount force-pushed the lines-cop-fix-wine-universal-binary branch from 84f2db8 to 85fa79b Compare October 15, 2017 14:37
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks @JCount!

@MikeMcQuaid MikeMcQuaid merged commit 1d40061 into Homebrew:master Oct 18, 2017
@JCount JCount deleted the lines-cop-fix-wine-universal-binary branch October 18, 2017 15:58
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants