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

Rubocop: 0.50.0 and Ruby 2.3 #3183

Merged
merged 3 commits into from Sep 25, 2017

Conversation

Projects
None yet
5 participants
@MikeMcQuaid
Member

MikeMcQuaid commented Sep 21, 2017

Also fix all cop renames, disable some cops and fix all warnings.

Want to also fix any/all Homebrew/core warnings first before merging this.

Show outdated Hide outdated Library/Homebrew/brew.rb Outdated
@@ -163,7 +163,7 @@ def elisp_caveats
def plist_caveats
s = []
if f.plist || (keg && keg.plist_installed?)
if f.plist || (keg&.plist_installed?)

This comment has been minimized.

@ilovezfs

ilovezfs Sep 21, 2017

Contributor

are the parentheses still needed?

@ilovezfs

ilovezfs Sep 21, 2017

Contributor

are the parentheses still needed?

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Sep 21, 2017

Member

Removed.

@MikeMcQuaid

MikeMcQuaid Sep 21, 2017

Member

Removed.

@@ -55,7 +55,7 @@ def prune
else
n, d = ObserverPathnameExtension.counts
print "Pruned #{n} symbolic links "
print "and #{d} directories " if d > 0
print "and #{d} directories " if d.positive?

This comment has been minimized.

@ilovezfs

ilovezfs Sep 21, 2017

Contributor

oh how gross. can we opt out of this?

@ilovezfs

ilovezfs Sep 21, 2017

Contributor

oh how gross. can we opt out of this?

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Sep 21, 2017

Member

Use Python?

@MikeMcQuaid

MikeMcQuaid Sep 21, 2017

Member

Use Python?

This comment has been minimized.

@ilovezfs

ilovezfs Sep 21, 2017

Contributor

you first

@ilovezfs

ilovezfs Sep 21, 2017

Contributor

you first

@JCount

JCount approved these changes Sep 21, 2017

Lint/RescueWithoutErrorClass:
Enabled: false
# implicitly allow EOS as we use it everywhere

This comment has been minimized.

@DomT4

DomT4 Sep 22, 2017

Contributor

Thanks for catching this. My $EDITOR's Rubocop plugin has already been driving me up the wall over EOS stuff.

Edit - Up the wall, not up to the wall.

@DomT4

DomT4 Sep 22, 2017

Contributor

Thanks for catching this. My $EDITOR's Rubocop plugin has already been driving me up the wall over EOS stuff.

Edit - Up the wall, not up to the wall.

This comment has been minimized.

@ilovezfs

ilovezfs Sep 22, 2017

Contributor

You can thank me for the fit I pitched about it lol

@ilovezfs

ilovezfs Sep 22, 2017

Contributor

You can thank me for the fit I pitched about it lol

This comment has been minimized.

@DomT4

DomT4 Sep 22, 2017

Contributor

I don't understand what the issue is with EOS to be honest. It'd seem silly to get ultra-specific inside formulae at least.

@DomT4

DomT4 Sep 22, 2017

Contributor

I don't understand what the issue is with EOS to be honest. It'd seem silly to get ultra-specific inside formulae at least.

This comment has been minimized.

@ilovezfs

ilovezfs Sep 22, 2017

Contributor

rubocop-hq/rubocop#4467

I get the sense that whatever conversations there were about this didn't really occur in the open.

@ilovezfs

ilovezfs Sep 22, 2017

Contributor

rubocop-hq/rubocop#4467

I get the sense that whatever conversations there were about this didn't really occur in the open.

This comment has been minimized.

@reitermarkus

reitermarkus Sep 23, 2017

Member

One nice thing is that in TextMate you get nested syntax highlighting inside heredocs if you use e.g. RUBY, but if it's just text, I'm also in favour of EOS.

@reitermarkus

reitermarkus Sep 23, 2017

Member

One nice thing is that in TextMate you get nested syntax highlighting inside heredocs if you use e.g. RUBY, but if it's just text, I'm also in favour of EOS.

@reitermarkus

This comment has been minimized.

Show comment
Hide comment
@reitermarkus

reitermarkus Sep 22, 2017

Member

You can bump HOMEBREW_RUBOCOP_CASK_VERSION to 0.14.2 now.

Member

reitermarkus commented Sep 22, 2017

You can bump HOMEBREW_RUBOCOP_CASK_VERSION to 0.14.2 now.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 23, 2017

Contributor

It's fitting that the only offense in all of core is

== Formula/gl2ps.rb ==
C: 61: 39: Use File.size("test.eps").positive? instead of File.size("test.eps") > 0.

4357 files inspected, 1 offense detected
Contributor

ilovezfs commented Sep 23, 2017

It's fitting that the only offense in all of core is

== Formula/gl2ps.rb ==
C: 61: 39: Use File.size("test.eps").positive? instead of File.size("test.eps") > 0.

4357 files inspected, 1 offense detected
@reitermarkus

This comment has been minimized.

Show comment
Hide comment
@reitermarkus

reitermarkus Sep 23, 2017

Member

RuboCop should at least suggest nonzero? in this case. 😜

Member

reitermarkus commented Sep 23, 2017

RuboCop should at least suggest nonzero? in this case. 😜

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 23, 2017

Contributor

git bisect led me here.

I'm getting

iMac-TMP:homebrew-core joe$ brew pull https://github.com/Homebrew/homebrew-core/pull/18474
==> Fetching patch 
Patch: https://github.com/Homebrew/homebrew-core/pull/18474.patch
######################################################################## 100.0%
==> Applying patch
Applying: libhttpseverywhere 0.6.0
Warning: libhttpseverywhere has a bottle: do you need to update it with --bottle?
==> Patch closes issue #18474
Warning: Nonstandard bump subject: libhttpseverywhere 0.6.0
Warning: Subject should be: libhttpseverywhere  (new formula)
==> Patch changed:
 Formula/libhttpseverywhere.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

which does not occur without this pulled.

The message

Warning: Subject should be: libhttpseverywhere  (new formula)

is bogus.

Contributor

ilovezfs commented Sep 23, 2017

git bisect led me here.

I'm getting

iMac-TMP:homebrew-core joe$ brew pull https://github.com/Homebrew/homebrew-core/pull/18474
==> Fetching patch 
Patch: https://github.com/Homebrew/homebrew-core/pull/18474.patch
######################################################################## 100.0%
==> Applying patch
Applying: libhttpseverywhere 0.6.0
Warning: libhttpseverywhere has a bottle: do you need to update it with --bottle?
==> Patch closes issue #18474
Warning: Nonstandard bump subject: libhttpseverywhere 0.6.0
Warning: Subject should be: libhttpseverywhere  (new formula)
==> Patch changed:
 Formula/libhttpseverywhere.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

which does not occur without this pulled.

The message

Warning: Subject should be: libhttpseverywhere  (new formula)

is bogus.

@MikeMcQuaid MikeMcQuaid merged commit a589303 into Homebrew:master Sep 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MikeMcQuaid MikeMcQuaid deleted the MikeMcQuaid:rubocop-upgrade branch Sep 25, 2017

@commitay commitay referenced this pull request Sep 25, 2017

Closed

Travis CI #38950

@ilovezfs ilovezfs referenced this pull request Sep 26, 2017

Merged

bump-formula-pr: fix duplicates check #3214

3 of 5 tasks complete
@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Oct 8, 2017

Contributor

It seems 9eb51db broke the description audit. See #3286.

Contributor

ilovezfs commented Oct 8, 2017

It seems 9eb51db broke the description audit. See #3286.

ilovezfs added a commit to ilovezfs/brew that referenced this pull request Nov 2, 2017

formula: fix safe navigation bug
Safe navigation needs to be chained to preserve equivalence.

Fixes a bug introduced by 01e9ec9 in Homebrew#3183.

@ilovezfs ilovezfs referenced this pull request Nov 2, 2017

Merged

formula: fix safe navigation bug #3414

3 of 5 tasks complete

@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.