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

Travis-CI: fixes part2 #13

Merged
merged 12 commits into from Jan 10, 2017
Merged

Conversation

tschoonj
Copy link
Collaborator

@tschoonj tschoonj commented Jan 9, 2017

Please do not merge until the Travis-CI build is green

@tschoonj tschoonj merged commit cb958e0 into reinh-bader:master Jan 10, 2017
@tschoonj
Copy link
Collaborator Author

@reinh-bader I fixed the Travis-CI builds finally.

Also, I saw that you forgot to add the new nlfit.finc files to Makefile.am, which I took care of.

@reinh-bader
Copy link
Owner

OK, thanks.

@tschoonj
Copy link
Collaborator Author

If I may suggest a different workflow for non-trivial additions:

  1. create a new branch and check it out:
git checkout -b my-new-branch
  1. do some work
  2. commit
  3. push the new branch with the commit to github and track it
git push -u origin my-new-branch
  1. open a pull-request for this branch in the repository, and check the results of the Travis-CI builds: if one of them fails, fix the underlying cause in the code, commit and push, thereby triggering a new build
  2. for as long as you need to, keep making changes to the code, commit them and check the build results.
  3. when done, merge the PR in and delete the local and remote branch you did the work on.

Generally speaking, committing directly onto master is discouraged nowadays as it can be quite dangerous (git push --force!). In fact in the settings of the repository, you can even disable committing to master altogether by making it a so-called protected branch.

@reinh-bader
Copy link
Owner

Rather complicated. I'm game, though, except that I don't fully understand the following point:
The pull request goes to the branch, but in step 7. it is merged into the master - correct? Does the web interface support doing this?

Cheers

@tschoonj
Copy link
Collaborator Author

Absolutely, this is in fact what you did yesterday when you closed the PR by merging it in.

There are three ways of doing this (click in a future PR on the merge button to get the dropdown list):

  1. Merge: simplest way, creates a merge commit, even if the branch is already up to date with master
  2. Squash and merge: squashes all commits on the branch into a single commit, which is then merged into master, I think also with a merge commit. Useful if there's a series of small individual commits with only bug fixes. I did this in the PR I closed this morning.
  3. Rebase: rebase the branch on top of master and merge it in, but without a merge commit.

@reinh-bader
Copy link
Owner

OK, thanks. The next commit will still go to master (I've already done some changes), but I'll start using the new workflow after that. I'll also document that workflow in README_WORKFLOW, because after a long interval of inactivity I'm bound to forget about it :-/

@tschoonj
Copy link
Collaborator Author

Actually, assuming you haven't made any commits locally on master that you haven't pushed yet, you will first need to pull in my latest commit with the Travis-CI changes locally with a simple git pull.

Otherwise you will need to rebase your local master against the remote master (check first that you are on master with git status, just in case 😄 ):

git fetch
git rebase origin/master
git push

Also, you can make a new branch whenever you want, even when there are already modified files.

Another advantage of not working on master directly is that it frees you from having to push your changes immediately to github, as it is always possible that someone has pushed commits to master already!

@reinh-bader
Copy link
Owner

Hmm, the branches CI tests seem to have problems with the MAC builds, However, it seems that the build doesn't start up properly. Linux builds are fine.

@tschoonj
Copy link
Collaborator Author

Well I managed to get them right in the end 😄

Actually the main problem was that the new nlfit.finc files had not been added to Makefile.am, which caused make distcheck to fail. This took me quite some time to figure out 😄

@reinh-bader
Copy link
Owner

The trouble is that I can't see any messages on the console. If I try to open it, I only get the message
"Hang tight, the log cannot be shown until the build has started."
I will try to check "make distcheck" on a separate system tomorrow (on the devel system, there is some trouble with tar due to my high GID on that system).

@tschoonj
Copy link
Collaborator Author

You mean the Mac OS X builds right? See my latest comment in #15

@tschoonj tschoonj deleted the Travis-CI-fixes branch February 17, 2017 14:06
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.

None yet

2 participants