Skip to content

Comments

Only run Travis for the master branch & run sanity checks with DMD too#441

Merged
PetarKirov merged 2 commits intodlang-tour:masterfrom
wilzbach:travis-magic
Aug 14, 2016
Merged

Only run Travis for the master branch & run sanity checks with DMD too#441
PetarKirov merged 2 commits intodlang-tour:masterfrom
wilzbach:travis-magic

Conversation

@wilzbach
Copy link
Member

A bit more of Travis magic applied:

  1. restrict only to the master branch (currently it runs twice with all PRs made with branches on dlang-tour, e.g. hard to avoid when coming from the web UI)
  2. Run sanity checks with for DMD too (just in case there are some recent compiler bugs)
  3. Since quite some time we use ldc instead of gdc to create the static binary -> fixed the comment

@stonemaster
Copy link
Collaborator

👍

.travis.yml Outdated
# Compiler to static binary with gdc
- if [[ "${DC}" == "dmd" ]]; then dub test --compiler=${DC}; fi
- if [[ "${DC}" == "dmd" ]]; then dub --compiler=${DC} -- --sanitycheck; fi
# Compiler to static binary with ldc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo - I think it should be "Compile to ...".

@wilzbach
Copy link
Member Author

This leads to about 30 seconds increase in the run time of the DMD build, but as it's still faster than LDC, I think we don't need to care ;-)

@PetarKirov
Copy link
Member

Apart from small nit, LGTM. Feel free to merge when ready.

@PetarKirov PetarKirov merged commit 8bcb160 into dlang-tour:master Aug 14, 2016
@wilzbach wilzbach deleted the travis-magic branch August 14, 2016 21:33
@PetarKirov
Copy link
Member

PetarKirov commented Aug 14, 2016

I tried the new squash feature of GitHub and looks like it works good - it added to the commit message a link to this PR.

@wilzbach
Copy link
Member Author

LGTM. Feel free to merge when ready.

Thanks & good catch!

I tried the new squash feature of GitHub and looks like it works good

If we like it, we should be able to make it the default (by disallowing normal merge commits). This way I can at least not accidentally press on "merge" on my phonem :D

Btw a minor thing that i noticed - it copies all commit messages in the description (like the usual git squash). I think in general this is nice, just be careful when someone uses weird commit messages.
A good, negative example from Phobos:

dlang/phobos#4662

it added to the commit message a link to this PR.

I think it does this by default for merge commits too:

214a833

@PetarKirov
Copy link
Member

A good, negative example from Phobos

Yeah, I remember that one. I guess that guy could have squashed his commits into an "appeasing the git Nazis" message :D

Btw a minor thing that i noticed - it copies all commit messages in the description (like the usual git squash). I think in general this is nice, just be careful when someone uses weird commit messages.

The good thing is that it opens a textbox where you can manually edit the commit message. I just decided to leave it as it is for this PR.

I think it does this by default for merge commits too

I already new that it does that for merge messages, I was just worried that we might loose the PR context if squashing was used. C.f. Vladimir's post on the forum.

If we like it, we should be able to make it the default (by disallowing normal merge commits).

I don't think this would be a good idea, because it would make larger PRs harder to review, which usually are not if the they're built from a number of small commits.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants