-
Notifications
You must be signed in to change notification settings - Fork 58
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
Working deploy with travis-ci.com for the RaspberryPi3 (arm64) support #35
Conversation
…t generated a valid deploy key, hopefully. This time I should get past this: https://travis-ci.org/github/CCHS-Melbourne/openpnp-capture/jobs/679305130#L1124
…e deployed on openpnp/openpnp-capture
Works on XCode 11.4.1 locally, although might need some tweaks on
|
Oh dear, it turns out XCode 11.4 is fine, but TravisCI still does not support Catalina?: https://travis-ci.community/t/macos-catalina-build-environment/5608/16 https://docs.travis-ci.com/user/reference/osx/#macos-version ... sigh Possible OS-version-dependent fix spotted here: https://github.com/opencv/opencv/pull/15517/files |
… announcement: https://changelog.travis-ci.com/xcode-11-4-released-144748 ... also, weirdly enough, https://travis-ci.org/github/openpnp/openpnp-capture/jobs/676990772#L7 and https://travis-ci.org/github/brainstorm/openpnp-capture/jobs/681454101#L7 are assigned older MacOS TravisCI workers and accompanying OS and software :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @brainstorm - overall I'm happy to merge this, but there are a number of changes to the .travis.yml that don't seem to be needed. I'm curious if these are these made on purpose, or if a tool made the changes. If the changes aren't required for your deployment I'd prefer if they are reverted.
deploy: | ||
provider: releases | ||
api_key: | ||
token: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this change? I don't see "token" documented on https://docs.travis-ci.com/user/deployment/releases/
file: deploy/* | ||
skip_cleanup: true | ||
file: deploy/* | ||
cleanup: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above: Why is this changed? "cleanup" does not seem to be documented.
skip_cleanup: true | ||
file: deploy/* | ||
cleanup: false | ||
edge: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, it looks like this selects the beta version of the provider, which explains the above questions. Is this required for your deployment to work? If it's not, I don't see a good reason to switch to a beta provider at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all the changes above are informed by TravisCI support, travis lint
's CLI and web linter too:
https://config.travis-ci.com/explore
See how your current .travis.yml
on your master
looks like against the official linter:
I really just wanted to future-proof this a little bit, but happy to revert back to the old syntax.
on: | ||
all_branches: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to create a new release for every branch. Is there some reason this is added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was added since I was getting "Not deploying because rpi3_arm64
is not an authorized branch". In my experience, I've found that when combined with tags: true
, it conserves the original behaviour (only release on tags) and at the same time, it allows for easier deployment testing on forks (my case), since I didn't want to pollute my forked master
to test whether it deployed well or not. Happy to remove it anyway now that the work is done.
Thanks @brainstorm - all sounds good. I appreciate the detailed response. Let's give it a try! |
This is a followup on the failed deploy for PR #32.
The underlying issue has been resolved by travisci support folks, as explained/tracked on twitter:
https://twitter.com/braincode/status/1253981272885477376
This pullrequest should now deploy, as demonstrated on this fork:
https://github.com/brainstorm/openpnp-capture/releases
and
https://travis-ci.com/github/brainstorm/openpnp-capture/builds/162954136
As long as this particular repository is migrated to travis-ci.com instead of travis-ci.org (change to be operated by @vonnieda):
https://docs.travis-ci.com/user/migrate/open-source-repository-migration