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

Rename tap-readme command to tap-new. #1277

Merged
merged 2 commits into from Oct 15, 2016

Conversation

Projects
None yet
2 participants
@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Oct 12, 2016

  • 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?

Write the README but also a .travis.yml file (and in future perhaps a
Jenkinsfile).

@scpeters

This comment has been minimized.

Copy link
Contributor

scpeters commented Oct 12, 2016

Thanks, I didn't have a chance to get around to this. I'm testing the recommended travis.yml as we speak

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Oct 13, 2016

@scpeters Let me know when you're tried it out.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:tap-new-command branch from d53561f to 7cbef97 Oct 13, 2016

@scpeters

This comment has been minimized.

Copy link
Contributor

scpeters commented Oct 13, 2016

I've tried it out, and I added a matrix so that I could run both 10.11 and 10.10 in travis since we only have one CI machine building bottles, and it's still on 10.10.

I tried pushing a style error to a formula in scpeters/homebrew-simulation@ec18af3 but I guess test-bot doesn't check for style errors. We have some formulae that take a long time to compile, so I think that's why I opted for an alternate travis file in the past. I found a weird error in one of our bottles (Error: invalid byte sequence in UTF-8) with this build, so I need to investigate that now, but I think it's unrelated to this travis script.

I'll try another formula modification and see what happens.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:tap-new-command branch from 7cbef97 to a94e3f6 Oct 13, 2016

@scpeters

This comment has been minimized.

Copy link
Contributor

scpeters commented Oct 13, 2016

I bumped a formula revision in scpeters/homebrew-simulation@51eb4fe and both matrix builds timed out after 45 minutes. A similar build takes about 20 minutes on our multi-core mac mini, so I guess travis is not enough for these formulae.

I'll see if there's any test-bot args to skip the bottle build.

As an aside, we are working to split this monolithic project into separate packages and formulae, but that is a work in progress.

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Oct 13, 2016

I bumped a formula revision in scpeters/homebrew-simulation@51eb4fe and both matrix builds timed out after 45 minutes. A similar build takes about 20 minutes on our multi-core mac mini, so I guess travis is not enough for these formulae.

Yes, there's nothing you can really do to make this build faster and as a result Travis is not useful for building huge things on macOS.

@scpeters

This comment has been minimized.

Copy link
Contributor

scpeters commented Oct 13, 2016

I think his PR is good for the general case, though the travis script doesn't work well for my tap at the moment.

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Oct 13, 2016

What do you do in your tap instead that works for you, out of interest?

@scpeters

This comment has been minimized.

Copy link
Contributor

scpeters commented Oct 13, 2016

https://github.com/osrf/homebrew-simulation/blob/master/.travis.yml#L26-L29

  - brew audit `ls $(brew --repo)/Library/Taps/osrf/homebrew-simulation/*.rb | grep -v sofa.rb`
  - brew style `ls $(brew --repo)/Library/Taps/osrf/homebrew-simulation/*.rb`
  - brew fetch --force-bottle `ls $(brew --repo)/Library/Taps/osrf/homebrew-simulation/*.rb | grep -v sofa.rb`
  - brew unpack --patch `ls $(brew --repo)/Library/Taps/osrf/homebrew-simulation/*.rb | grep -v sofa.rb`

It runs audit, style, fetch and verifies that patches apply cleanly for all formulae in the tap (except one that his head-only). The tap is small enough that these operations fit in the travis build window.

I wouldn't mind if these operations were done only on the formula touched by a pull request, but I wasn't sure how to do that.

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Oct 14, 2016

@scpeters You can do that for the changed formulae in a PR with --fast which will avoid building but just check checksums/audit.

- ulimit -n 1024
script:
- brew test-bot

This comment has been minimized.

@scpeters

scpeters Oct 14, 2016

Contributor

Now that I look at it more closely, I wonder if we need --tap=${tap}, since homebrew-science is using that?

This comment has been minimized.

@scpeters

scpeters Oct 14, 2016

Contributor

never mind, it should be auto-detected...

MikeMcQuaid added some commits Oct 13, 2016

Rename tap-readme command to tap-new.
Write the README but also a `.travis.yml` file (and in future perhaps a
`Jenkinsfile`).

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:tap-new-command branch from a94e3f6 to 7ab39e1 Oct 15, 2016

@MikeMcQuaid MikeMcQuaid merged commit 4482234 into Homebrew:master Oct 15, 2016

4 checks passed

codecov/patch 100% of diff hit (target 63.02%)
Details
codecov/project 63.04% (+0.01%) compared to 1327640
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Build finished.
Details

@MikeMcQuaid MikeMcQuaid deleted the MikeMcQuaid:tap-new-command branch Oct 15, 2016

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

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