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

linkage_checker: add a check for extraneous dependencies #3103

Merged
merged 4 commits into from Sep 18, 2017

Conversation

Projects
None yet
5 participants
@maxim-belkin
Contributor

maxim-belkin commented Aug 29, 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?

Detect extraneous dependencies in formulae. Might be useful for new formulae.

  • Have not written tests for my changes. Suggestions what tests to write are welcome
  • brew tests fail for some other tests
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 29, 2017

Member

I'm not sure I'm reading this right but is an extraneous dependency any where there's no linkage? If so, this will need to be an optional flag as there will be a lot of false positives otherwise (i.e. anything that calls an executable from a dependency rather than linking against it, most scripting language dependencies, etc.)

Member

MikeMcQuaid commented Aug 29, 2017

I'm not sure I'm reading this right but is an extraneous dependency any where there's no linkage? If so, this will need to be an optional flag as there will be a lot of false positives otherwise (i.e. anything that calls an executable from a dependency rather than linking against it, most scripting language dependencies, etc.)

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 29, 2017

Contributor

A lot of false positives could be eliminated by ignoring any keg that has a bin directory. Many packages whose primary purpose is to provide a library have include and lib directories, but no bin directory.

Contributor

sjackman commented Aug 29, 2017

A lot of false positives could be eliminated by ignoring any keg that has a bin directory. Many packages whose primary purpose is to provide a library have include and lib directories, but no bin directory.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Aug 29, 2017

Contributor

I agree: false positives are a possibility.

brew linkage ffmpeg

reports xvid as a possible extraneous dependency

Contributor

maxim-belkin commented Aug 29, 2017

I agree: false positives are a possibility.

brew linkage ffmpeg

reports xvid as a possible extraneous dependency

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Aug 29, 2017

Contributor

Those interested in testing this, try the following "one-liner" to check your installed formulae:

for formula in $(brew list); do echo "* Formula: $formula"; brew linkage $formula | sed -n '/^Possible/,/^[^ ]/p;'; done
Contributor

maxim-belkin commented Aug 29, 2017

Those interested in testing this, try the following "one-liner" to check your installed formulae:

for formula in $(brew list); do echo "* Formula: $formula"; brew linkage $formula | sed -n '/^Possible/,/^[^ ]/p;'; done
@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 29, 2017

Contributor

@ilovezfs identified that roughly half of depends_on "boost" dependencies in Homebrew/science use only the headers, and have no run-time linkage to boost, and so should in fact be depends_on "boost" => :build. This PR would help identify those cases. I'm having trouble finding the archived conversation—too many repos and slack channels. I agree entirely that the false positives have to be eliminated for this PR to be useful.

Contributor

sjackman commented Aug 29, 2017

@ilovezfs identified that roughly half of depends_on "boost" dependencies in Homebrew/science use only the headers, and have no run-time linkage to boost, and so should in fact be depends_on "boost" => :build. This PR would help identify those cases. I'm having trouble finding the archived conversation—too many repos and slack channels. I agree entirely that the false positives have to be eliminated for this PR to be useful.

@scpeters

This comment has been minimized.

Show comment
Hide comment
@scpeters

scpeters Aug 29, 2017

Contributor

@sjackman there could still be a need for boost to be installed for libraries that expose boost symbols in their own headers. Then anything that wants to build against the library would need the boost headers. It's kinda like the difference between lib* and lib*-dev packages on debian.

Contributor

scpeters commented Aug 29, 2017

@sjackman there could still be a need for boost to be installed for libraries that expose boost symbols in their own headers. Then anything that wants to build against the library would need the boost headers. It's kinda like the difference between lib* and lib*-dev packages on debian.

@scpeters

This comment has been minimized.

Show comment
Hide comment
@scpeters

scpeters Aug 29, 2017

Contributor

I just took a quick look at some of the packages in homebrew/science and found two that use the boost headers and install a library and header files (cantera and xylib). Despite having #include <boost/ statements in the installed header files, both of these formulae declare :build dependencies on boost, possibly because they also install executables and want to reduce the footprint for those users. If you're building from source against a library, they might be knowledgeable to install boost on their own.

Maybe these types of header-only dependencies that the linkage test can't see could be marked with :run like the build dependencies in audit?

https://github.com/Homebrew/brew/blob/master/Library/Homebrew/dev-cmd/audit.rb#L499-L507

Contributor

scpeters commented Aug 29, 2017

I just took a quick look at some of the packages in homebrew/science and found two that use the boost headers and install a library and header files (cantera and xylib). Despite having #include <boost/ statements in the installed header files, both of these formulae declare :build dependencies on boost, possibly because they also install executables and want to reduce the footprint for those users. If you're building from source against a library, they might be knowledgeable to install boost on their own.

Maybe these types of header-only dependencies that the linkage test can't see could be marked with :run like the build dependencies in audit?

https://github.com/Homebrew/brew/blob/master/Library/Homebrew/dev-cmd/audit.rb#L499-L507

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 29, 2017

Member

I agree: false positives are a possibility.

Given we're running this in CI for everything in Homebrew/homebrew-core: we really don't want false positives here.

@ilovezfs identified that roughly half of depends_on "boost" dependencies in Homebrew/science use only the headers, and have no run-time linkage to boost, and so should in fact be depends_on "boost" => :build. This PR would help identify those cases.

I agree. Having this as an optional flag which is always run for new formulae in brew test-botseems like a good idea.

Member

MikeMcQuaid commented Aug 29, 2017

I agree: false positives are a possibility.

Given we're running this in CI for everything in Homebrew/homebrew-core: we really don't want false positives here.

@ilovezfs identified that roughly half of depends_on "boost" dependencies in Homebrew/science use only the headers, and have no run-time linkage to boost, and so should in fact be depends_on "boost" => :build. This PR would help identify those cases.

I agree. Having this as an optional flag which is always run for new formulae in brew test-botseems like a good idea.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Aug 29, 2017

Contributor

Please suggest test-cases or directions how to improve the code

Contributor

maxim-belkin commented Aug 29, 2017

Please suggest test-cases or directions how to improve the code

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 30, 2017

Member

@maxim-belkin Use ARGV.include?("--something") and only enable the current functionality if that is passed.

Member

MikeMcQuaid commented Aug 30, 2017

@maxim-belkin Use ARGV.include?("--something") and only enable the current functionality if that is passed.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

@MikeMcQuaid The output of Possible undeclared dependencies is informative only. It doesn't affect the exit status of either brew test or brew test --missing. For example brew linkage awk reports Possible undeclared dependencies: gmp. Would that behaviour be an option for this feature as well? Enabled by default, print an informative message, but not affecting exit status. To make this feature useful, it's still important to eliminate as many false positives as possible.

Contributor

sjackman commented Aug 30, 2017

@MikeMcQuaid The output of Possible undeclared dependencies is informative only. It doesn't affect the exit status of either brew test or brew test --missing. For example brew linkage awk reports Possible undeclared dependencies: gmp. Would that behaviour be an option for this feature as well? Enabled by default, print an informative message, but not affecting exit status. To make this feature useful, it's still important to eliminate as many false positives as possible.

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 30, 2017

Member

It doesn't affect the exit status of either brew test or brew test --missing.

It doesn't affect the exit status or output of https://github.com/Homebrew/homebrew-test-bot/blob/80333911e7130b9542a6fe2b94c1311b52cef319/cmd/brew-test-bot.rb#L664 ? That's what we need to avoid.

Member

MikeMcQuaid commented Aug 30, 2017

It doesn't affect the exit status of either brew test or brew test --missing.

It doesn't affect the exit status or output of https://github.com/Homebrew/homebrew-test-bot/blob/80333911e7130b9542a6fe2b94c1311b52cef319/cmd/brew-test-bot.rb#L664 ? That's what we need to avoid.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

Sorry, I wrote too quickly and didn't engage brain. I had meant to say…
Possible undeclared dependencies doesn't affect the output status of brew linkage or brew linkage --test.
If the same were true for this PR Possible unnecessary dependencies would it be okay without an option to enable it?

Contributor

sjackman commented Aug 30, 2017

Sorry, I wrote too quickly and didn't engage brain. I had meant to say…
Possible undeclared dependencies doesn't affect the output status of brew linkage or brew linkage --test.
If the same were true for this PR Possible unnecessary dependencies would it be okay without an option to enable it?

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Aug 30, 2017

Contributor

I agree. Having this as an optional flag which is always run for new formulae in brew test-bot seems like a good idea.

Things that are known to give a lot of false positives aren't great even for --new-formula. We keep getting contributors asking, "Will my formula still qualify if those audits are failing?" and it's getting pretty old explaining that yes CI is bright red but no that doesn't matter because it will go away next time after it's merged.

Contributor

ilovezfs commented Aug 30, 2017

I agree. Having this as an optional flag which is always run for new formulae in brew test-bot seems like a good idea.

Things that are known to give a lot of false positives aren't great even for --new-formula. We keep getting contributors asking, "Will my formula still qualify if those audits are failing?" and it's getting pretty old explaining that yes CI is bright red but no that doesn't matter because it will go away next time after it's merged.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

This test will not affect the exit status of brew linkage --test, brew audit --new-formula, nor of CI. It'll only display an informational message, similar to Possible undeclared dependencies.

Contributor

sjackman commented Aug 30, 2017

This test will not affect the exit status of brew linkage --test, brew audit --new-formula, nor of CI. It'll only display an informational message, similar to Possible undeclared dependencies.

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 30, 2017

Member

@sjackman in that case: how does anyone notice that output and make changes as a result?

Member

MikeMcQuaid commented Aug 30, 2017

@sjackman in that case: how does anyone notice that output and make changes as a result?

@scpeters

This comment has been minimized.

Show comment
Hide comment
@scpeters

scpeters Aug 30, 2017

Contributor

I'm guessing it prints console messages but doesn't affect the exit status, which is what CI cares about.

Contributor

scpeters commented Aug 30, 2017

I'm guessing it prints console messages but doesn't affect the exit status, which is what CI cares about.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

Same as with the existing Possible undeclared dependencies message. It's only an informational message that does not affect CI status. If you ran brew linkage manually you'd see the message, or if you looked in the CI log, you'd see the message.

Contributor

sjackman commented Aug 30, 2017

Same as with the existing Possible undeclared dependencies message. It's only an informational message that does not affect CI status. If you ran brew linkage manually you'd see the message, or if you looked in the CI log, you'd see the message.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Aug 30, 2017

Contributor

Just an FYI: I've prepared a version with --extraneous so... just let me guys know what you decide. Personally, I think it is similar to undeclared dependencies, so I'd enable it by default.
As I mentioned in the OP, this could be used for new formulae: people could be asked to provide output of brew linkage when submitting a PR.

Contributor

maxim-belkin commented Aug 30, 2017

Just an FYI: I've prepared a version with --extraneous so... just let me guys know what you decide. Personally, I think it is similar to undeclared dependencies, so I'd enable it by default.
As I mentioned in the OP, this could be used for new formulae: people could be asked to provide output of brew linkage when submitting a PR.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

@sjackman in that case: how does anyone notice that output and make changes as a result?

Ah, I had forgotten that test-bot prints only the output of brew linkage --test and not brew linkage. In that case this PR does not affect CI at all. I would suggest modifying test-bot to run brew linkage, which always succeeds, in addition to brew linkage --test, so that the output of Possible undeclared dependencies and the proposed Possible unnecessary dependencies are in the build log.

+      test "brew", "linkage", dependent.name
       test "brew", "linkage", "--test", dependent.name

https://github.com/Homebrew/homebrew-test-bot/blob/master/cmd/brew-test-bot.rb#L664

Contributor

sjackman commented Aug 30, 2017

@sjackman in that case: how does anyone notice that output and make changes as a result?

Ah, I had forgotten that test-bot prints only the output of brew linkage --test and not brew linkage. In that case this PR does not affect CI at all. I would suggest modifying test-bot to run brew linkage, which always succeeds, in addition to brew linkage --test, so that the output of Possible undeclared dependencies and the proposed Possible unnecessary dependencies are in the build log.

+      test "brew", "linkage", dependent.name
       test "brew", "linkage", "--test", dependent.name

https://github.com/Homebrew/homebrew-test-bot/blob/master/cmd/brew-test-bot.rb#L664

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

Or brew linkage --test could be modified to keep its existing exit status behaviour, but print the more verbose message of brew linkage.

Contributor

sjackman commented Aug 30, 2017

Or brew linkage --test could be modified to keep its existing exit status behaviour, but print the more verbose message of brew linkage.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Aug 30, 2017

Contributor

Or brew linkage --test could be modified to keep its existing exit status behaviour, but print the more verbose message of brew linkage.

that's a possibility.

Contributor

maxim-belkin commented Aug 30, 2017

Or brew linkage --test could be modified to keep its existing exit status behaviour, but print the more verbose message of brew linkage.

that's a possibility.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 30, 2017

Contributor

The current output of brew linkage and brew linkage --test is…

❯❯❯ brew linkage gawk
System libraries:
  /usr/lib/libSystem.B.dylib
Homebrew libraries:
  /Users/sjackman/.homebrew/opt/gmp/lib/libgmp.10.dylib (gmp)
  /Users/sjackman/.homebrew/opt/mpfr/lib/libmpfr.4.dylib (mpfr)
  /Users/sjackman/.homebrew/opt/readline/lib/libreadline.7.dylib (readline)
Possible undeclared dependencies:
  gmp
❯❯❯ brew linkage --test gawk
No broken dylib links
Contributor

sjackman commented Aug 30, 2017

The current output of brew linkage and brew linkage --test is…

❯❯❯ brew linkage gawk
System libraries:
  /usr/lib/libSystem.B.dylib
Homebrew libraries:
  /Users/sjackman/.homebrew/opt/gmp/lib/libgmp.10.dylib (gmp)
  /Users/sjackman/.homebrew/opt/mpfr/lib/libmpfr.4.dylib (mpfr)
  /Users/sjackman/.homebrew/opt/readline/lib/libreadline.7.dylib (readline)
Possible undeclared dependencies:
  gmp
❯❯❯ brew linkage --test gawk
No broken dylib links
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 31, 2017

Member

Or brew linkage --test could be modified to keep its existing exit status behaviour, but print the more verbose message of brew linkage.

This seems sensible. That said: how are people going to notice this for formulae in CI or is this intended to just be manually run by maintainers? I'm trying to understand the use case.

Member

MikeMcQuaid commented Aug 31, 2017

Or brew linkage --test could be modified to keep its existing exit status behaviour, but print the more verbose message of brew linkage.

This seems sensible. That said: how are people going to notice this for formulae in CI or is this intended to just be manually run by maintainers? I'm trying to understand the use case.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Aug 31, 2017

Contributor

Of 47 formulae that depend on boost in Homebrew/science, @ilovezfs identified that 20 of those 47 formulae have no library linkage to boost (that is, they use only the headers), so should in fact be depends_on "boost" => :build rather than depends_on "boost". This PR would allow a developer (maintainer or contributor) to run brew linkage foo, see the message Possible unnecessary dependency: boost, and change the dependency to a :build dependency. I would use this command when creating a new formula, or troubleshooting existing formulae.
Homebrew/homebrew-science#6192 (comment)

Contributor

sjackman commented Aug 31, 2017

Of 47 formulae that depend on boost in Homebrew/science, @ilovezfs identified that 20 of those 47 formulae have no library linkage to boost (that is, they use only the headers), so should in fact be depends_on "boost" => :build rather than depends_on "boost". This PR would allow a developer (maintainer or contributor) to run brew linkage foo, see the message Possible unnecessary dependency: boost, and change the dependency to a :build dependency. I would use this command when creating a new formula, or troubleshooting existing formulae.
Homebrew/homebrew-science#6192 (comment)

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 1, 2017

Contributor
  1. extraneous or unnecessary?
  2. what should I do?
    1. leave the PR "as is" 😊
    2. move extraneous/unnecessary functionality under the --test
      1. move undeclared dependencies under --test
Contributor

maxim-belkin commented Sep 1, 2017

  1. extraneous or unnecessary?
  2. what should I do?
    1. leave the PR "as is" 😊
    2. move extraneous/unnecessary functionality under the --test
      1. move undeclared dependencies under --test
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 1, 2017

Member

This PR would allow a developer (maintainer or contributor) to run brew linkage foo, see the message Possible unnecessary dependency: boost, and change the dependency to a :build dependency.

That seems like a good workflow but I guess I'm just wondering if this is reliant on individual developers to manually do this: is it going to be useful? I see no harm in this PR as-is being merged, to be clear, I'm just trying to figure out if there's some better workflow.

Member

MikeMcQuaid commented Sep 1, 2017

This PR would allow a developer (maintainer or contributor) to run brew linkage foo, see the message Possible unnecessary dependency: boost, and change the dependency to a :build dependency.

That seems like a good workflow but I guess I'm just wondering if this is reliant on individual developers to manually do this: is it going to be useful? I see no harm in this PR as-is being merged, to be clear, I'm just trying to figure out if there's some better workflow.

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 1, 2017

Member

extraneous or unnecessary?

unnecessary

what should I do?

As long as the exit code isn't modified I'm happy as-is. It does seem like it would be useful to have this under --test so test-bot runs it.

Member

MikeMcQuaid commented Sep 1, 2017

extraneous or unnecessary?

unnecessary

what should I do?

As long as the exit code isn't modified I'm happy as-is. It does seem like it would be useful to have this under --test so test-bot runs it.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 2, 2017

Contributor

@maxim-belkin I'd suggest Possible build or unnecessary dependencies

Contributor

sjackman commented Sep 2, 2017

@maxim-belkin I'd suggest Possible build or unnecessary dependencies

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 2, 2017

Contributor

I'm just trying to figure out if there's some better workflow.

If it were possible to eliminate all the false positives, then it could affect the brew linkage --test exit status. It may be possible, but it would require significant work, and may in fact be impossible to get 100% perfect. If it got to the point of being mostly correctly (but not entirely), perhaps it could be enabled as a check for new formula only.

We could add a tickbox to .github/PULL_REQUEST_TEMPLATE.md that asks the user to run brew linkage foo and consider whether any of the Possible build or unnecessary dependencies may in fact be a :build dependency or unnecessary.

Contributor

sjackman commented Sep 2, 2017

I'm just trying to figure out if there's some better workflow.

If it were possible to eliminate all the false positives, then it could affect the brew linkage --test exit status. It may be possible, but it would require significant work, and may in fact be impossible to get 100% perfect. If it got to the point of being mostly correctly (but not entirely), perhaps it could be enabled as a check for new formula only.

We could add a tickbox to .github/PULL_REQUEST_TEMPLATE.md that asks the user to run brew linkage foo and consider whether any of the Possible build or unnecessary dependencies may in fact be a :build dependency or unnecessary.

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 4, 2017

Member

unnecessary

Let's go for this and then we can merge as-is if it's useful to you like that.

Member

MikeMcQuaid commented Sep 4, 2017

unnecessary

Let's go for this and then we can merge as-is if it's useful to you like that.

@sjackman

This comment has been minimized.

Show comment
Hide comment
@sjackman

sjackman Sep 5, 2017

Contributor

I'd suggest also renaming the code extraneous_deps to unnecessary_deps in that case, to match.

Contributor

sjackman commented Sep 5, 2017

I'd suggest also renaming the code extraneous_deps to unnecessary_deps in that case, to match.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 5, 2017

Contributor
Contributor

maxim-belkin commented Sep 5, 2017

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 5, 2017

Contributor

In my second implementation of this PR (which I'm abandoning), I had to factor out declared_dep_names method from check_undeclared_deps

I can do that here too, if it seems reasonable/necessary.

Contributor

maxim-belkin commented Sep 5, 2017

In my second implementation of this PR (which I'm abandoning), I had to factor out declared_dep_names method from check_undeclared_deps

I can do that here too, if it seems reasonable/necessary.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 5, 2017

Contributor

I wonder if we should report possible unnecessary dependencies (PUDs) with bin directories as "tier-2" PUDs

Contributor

maxim-belkin commented Sep 5, 2017

I wonder if we should report possible unnecessary dependencies (PUDs) with bin directories as "tier-2" PUDs

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 5, 2017

Contributor

I think the linkage_checker can be improved further. Code that detects undeclared/unnecessary dependencies is executed under --test, when, in fact, it should not.

link

Contributor

maxim-belkin commented Sep 5, 2017

I think the linkage_checker can be improved further. Code that detects undeclared/unnecessary dependencies is executed under --test, when, in fact, it should not.

link

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 6, 2017

Contributor

can someone reinitiate the build? I think there was an intermittent issue

Contributor

maxim-belkin commented Sep 6, 2017

can someone reinitiate the build? I think there was an intermittent issue

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 6, 2017

Contributor

Those errors are currently affecting most things 🙈

Contributor

ilovezfs commented Sep 6, 2017

Those errors are currently affecting most things 🙈

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 6, 2017

Contributor

💩

Contributor

maxim-belkin commented Sep 6, 2017

💩

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 6, 2017

Contributor

Could someone have a look at the following lines of this PR more closely: line 8 and 125 through 127.

Contributor

maxim-belkin commented Sep 6, 2017

Could someone have a look at the following lines of this PR more closely: line 8 and 125 through 127.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 11, 2017

Contributor

@maxim-belkin you need to rebase the PR or it will keep failing.

Contributor

ilovezfs commented Sep 11, 2017

@maxim-belkin you need to rebase the PR or it will keep failing.

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 11, 2017

Contributor

it did not fail - it did not run one of the tests. but I'll rebase the PR.

Contributor

maxim-belkin commented Sep 11, 2017

it did not fail - it did not run one of the tests. but I'll rebase the PR.

maxim-belkin added some commits Aug 29, 2017

linkage_checker: unnecessary dependencies
- rename 'extraneous' to 'unnecessary'
- add the report under `linkage --test`
@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 12, 2017

Contributor

any suggestion how to improve test fix "codecov/patch"?

Contributor

maxim-belkin commented Sep 12, 2017

any suggestion how to improve test fix "codecov/patch"?

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 18, 2017

Member

Looks good to me, nice work @maxim-belkin!

Member

MikeMcQuaid commented Sep 18, 2017

Looks good to me, nice work @maxim-belkin!

@MikeMcQuaid MikeMcQuaid merged commit 81d9f71 into Homebrew:master Sep 18, 2017

2 of 3 checks passed

codecov/patch 50% of diff hit (target 67.06%)
Details
codecov/project 67.07% (+0.01%) compared to f3ec40d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxim-belkin maxim-belkin deleted the maxim-belkin:extraneous_deps branch Sep 18, 2017

@maxim-belkin

This comment has been minimized.

Show comment
Hide comment
@maxim-belkin

maxim-belkin Sep 18, 2017

Contributor

Glad to contribute!

Contributor

maxim-belkin commented Sep 18, 2017

Glad to contribute!

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