Skip to content
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

Improve travis-ci.sh #1833

Merged
merged 2 commits into from
Dec 30, 2019
Merged

Improve travis-ci.sh #1833

merged 2 commits into from
Dec 30, 2019

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Dec 23, 2019

This will probably break horribly, but there's only one way to find out...

@dlang-bot
Copy link
Collaborator

Thanks for your pull request, @Geod24!

Put the 'regular' test code at the beginning,
and the part that relies on COVERAGE being defined only once at the end.
Those could eventually be split in the future.
@Geod24 Geod24 force-pushed the scripts-replacement branch from 3ce4866 to 2134119 Compare December 23, 2019 07:24
@Geod24
Copy link
Member Author

Geod24 commented Dec 23, 2019

Ugh this stage setup is also terrible. If something fails in the first stage and is retriggered (and pass), the second stage never starts so the whole build is marked as 'cancelled' and one has to restart everything.

Also the reason why #1795 happens is pretty clear, looking at .travis-ci.yaml...
@wilzbach : Since you are our resident Azure pipeline expert, do you think you'll have some time to do your magic before the end of the month ? Otherwise I might take a look at it.

@wilzbach
Copy link
Member

wilzbach commented Dec 23, 2019

@wilzbach : Since you are our resident Azure pipeline expert, do you think you'll have some time to do your magic before the end of the month ? Otherwise I might take a look at it.

Yeah it was on my list already. I will be travelling a lot for the next few days, but I think I can get this up before New Year's Eve.

@wilzbach
Copy link
Member

Ugh this stage setup is also terrible. If something fails in the first stage and is retriggered (and pass), the second stage never starts so the whole build is marked as 'cancelled' and one has to restart everything

BTW the reason for running the second stage for every PR was that once we enabled the Travis builds which upload to GitHub we weren't sure how reliable they are and it was also the only way to test the PRs. However, a solution would be to enable them only for a PR which is from the dub repository and not with branches from forks.

@Geod24
Copy link
Member Author

Geod24 commented Dec 23, 2019

Travis builds which upload to GitHub

I don't quite get why we do that ?
Those 6 builds are pretty much useless right now. The OSX one is useful, but it should be part of test. They also use a different set of flags (release flags) but that could be just 1 entry in the test matrix.

IIUC, the original intent was for this stage to trigger only on tags:

dub/.travis.yml

Lines 105 to 107 in 2e9a26a

# Until deployment of the release binaries is fixed, always build them
#- name: deploy
#if: type = push and tag =~ ^v\d+\.\d+\.\d+[^-]*\$ # not a pre-release tag

Which makes much more sense to me. I couldn't find much in the original PR. I guess whatever bug you were working around at the time might have been worked out, since LTO has seen a lot of love over the last 2 years ?

@wilzbach
Copy link
Member

I don't quite get why we do that ?

So that the people can download dub binaries (from GitHub releases). This is interesting for example if you want to have a newer version of dub with an older compiler.

IIUC, the original intent was for this stage to trigger only on tags:

Yes, though as I mentioned this wasn't done as otherwise the scripts couldn't be tested. I fully agree that there should be no need to run them for a normal PR build. I was just explaining the historical reason ... or at least trying to.

I guess whatever bug you were working around at the time might have been worked out, since LTO has seen a lot of love over the last 2 years ?

Yes.

@Geod24
Copy link
Member Author

Geod24 commented Dec 24, 2019

Okay, I think revisiting the travis config can be done later though. This is good to go as is and already gave me useful insight.

@Geod24 Geod24 requested a review from thewilsonator December 30, 2019 10:23
@dlang-bot dlang-bot merged commit 14cc8e5 into dlang:master Dec 30, 2019
@Geod24 Geod24 deleted the scripts-replacement branch December 30, 2019 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants