CI: reduce git load #5490
CI: reduce git load #5490
Conversation
| if [ ! -z "$CI_GITHUB_API_KEY" ]; then | ||
| if [ -z "$CI_GITHUB_API_KEY" ]; then | ||
| echo "curl: API key not present, exit with no error" | ||
| exit 0 |
There was a problem hiding this comment.
I think that not preparing the package and exiting with 0 exit code is a bad behavior for the script. If I run it locally, it should either generate a package or exit with an error.
Skipping this script if the API key is not set should be handled by if: env(CI_GITHUB_API_KEY) IS present in travis yml. If you have observed that that does not work, it should be fixed there, I think.
There was a problem hiding this comment.
Nevermind, I see now the link to the failed job in gitter. It seems that the condition in the yml does not work correctly, as variables get removed after the job has started. I think we need some other condition there, like fork = false...
There was a problem hiding this comment.
No more condition in yml.
- In a PR, package script is verbose and does not fail whether or not curl is failing.
- In a branch, curl failing makes the job failed.
|
Also need to push the same branch to esp8266/Arduino and check that CI passes, before merging. |
|
d-a-v approved this pull request.
Countless tests above were always made simultaneously in a PR and in a branch. No more "Travis failed with some weird error" |
| if [ -z "$CI_GITHUB_API_KEY" ]; then | ||
| # github api is rate-limiting | ||
| echo "---- Bad moon phase, in a PR, exit successfully" | ||
| exit 0 |
There was a problem hiding this comment.
My comment above about failing with non-zero exit code is still valid, I think. If this is started from command line, it should not pretend to succeed without doing anything useful.
There was a problem hiding this comment.
@igrr my (limited) understanding here is as follows:
- when run from the main repo, this will always work.
- when in a forked PR, the CI run may or may not hit the api limit. With this change, if the limit is hit, it won't cause the run to fail, hence still allowing a merge.
- Without this change, if the limit is hit, the run will fail, which leaves as only choice restarting the job.
- Skipping the deploy job based on the existence of the key apparently doesn't work.
Assuming the above is correct, what alternative is there?
|
requested changes addressed in #5496 |
no more dependency on savannah.nongnu.org nor bearssl.org