5 ms·

I prefer the concept that master should always be deployable to production. My CI creates a build artifact on every commit to master and the artifact can be de

by Osiris 11y ago

I prefer the concept that master should always be deployable to production. My CI creates a build artifact on every commit to master and the artifact can be deployed to staging/production at any time. Thus, master should always be considered stable. Development should happen in a branch. When it's done, checkout master LOCALLY, merge in your branch, run unit tests, if tests pass, push master. If tests don't pass, reset master back to origin and keep working on your feature. This way master is always ready to go and you also get the benefit of seeing how a feature was developed and the merge commit is a representation of a complete chunk of work. If you do need to back out a feature, how would you do it on a rebased, fast-forward merge? You have NO CLUE where the start of the feature was. You have to just guess based on the author maybe? In my experience, I've never used git bisect so I don't optimize my branching model for that experience. For me, backing out bad merges is much more common so I optimize for that.

killercup 11y ago
That's how I like to work as well. For libraries/open source, homu is quite a nice gatekeeper to pushing to master (you basically tell the bot to integrate a PR into master, and it runs the tests on CI for you before pushing). [1]: http://homu.io http://homu.io
- lisivka 11y ago
  IMHO, semaphoreci is better: it runs build without need to write anything in PR, so no additional teaching is required.
nthj 11y ago
> If you do need to back out a feature, how would you do it on a rebased, fast-forward merge? I've never been on a team where we fast-forward-merged multiple commits as a feature. (I'm sure it happens, but I can't see it working well.) What I have seen is fast-forward-merging a single commit (which is equivalent to a cherry-pick, at that point), after using git rebase to squash the feature branch into one single atomic commit and rebasing that commit against master in preparation for mainlining. Then, you'll never have merge conflicts (if you do, you didn't rebase against master properly prior to the merge), and if we need to revert a feature, the feature lives in master as a single commit.
- 0x0 11y ago
  Squashing a large feature branch for rebase has a downside that it loses some history and timeline context; if for example an api changed slightly in master in between branch development and branch squash, it can be very confusing trying to understand why the commit did XYZ thing looking at the squashed diff - while if you keep the branch through a merge, it's easy to see that XYZ was written with the older API in mind.
  - Osiris 11y ago
    This is exactly why I prefer merge commits, so you can see how the development of the feature occurred and if there's a bug where/when/how it was introduced. I don't like erasing development history for the sake of a "clean" or "linear" history.
bsimpson 11y ago
A nice side-effect of having master===production is that your documentation on GitHub (or whatever web viewer you use) is always consistent with the deployed version. Otherwise, you could have users come to your repo looking for docs for the version they've installed, and instead get docs about APIs that aren't released yet.
sciurus 11y ago
What you describe for testing and merging is a needlessly manual process. Using e.g. Phabricator, I can make a local feature branch and when I think it's ready open a request for code review. My CI system can run tests. I can update my request in response to test failures or reviewers comments. When my code is ready, Phabricator can squash all my commits into one and rebase that onto master. If for whatever reason I did want to back out this code, there's a single commit to revert. This is unlikely though, since until a feature has proven itself in production it will be behind a feature flag. https://secure.phabricator.com/book/phabflavor/article/recommendations_on_branching/ https://secure.phabricator.com/book/phabflavor/article/recom...
- Osiris 11y ago
  That's a nice process. Currently, GitLab doesn't support running tests on a merge request in that sense, making a temporary merge of master and the branch and running the tests to confirm the merge will work. When/If GitLab + GitLab CI adds that ability, then yes, it would make that process a lot easier.
  - sytse 11y ago
    We're discussing adding a 'test the merge' ability in 8.7 in https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 This will test the 'current merge' as soon as a commit is pushed, not the merge result when it happens.

by Osiris 11y ago

killercup 11y ago

That's how I like to work as well. For libraries/open source, homu is quite a nice gatekeeper to pushing to master (you basically tell the bot to integrate a PR into master, and it runs the tests on CI for you before pushing). [1]: http://homu.io http://homu.io

lisivka 11y ago
IMHO, semaphoreci is better: it runs build without need to write anything in PR, so no additional teaching is required.

nthj 11y ago

> If you do need to back out a feature, how would you do it on a rebased, fast-forward merge? I've never been on a team where we fast-forward-merged multiple commits as a feature. (I'm sure it happens, but I can't see it working well.) What I have seen is fast-forward-merging a single commit (which is equivalent to a cherry-pick, at that point), after using git rebase to squash the feature branch into one single atomic commit and rebasing that commit against master in preparation for mainlining. Then, you'll never have merge conflicts (if you do, you didn't rebase against master properly prior to the merge), and if we need to revert a feature, the feature lives in master as a single commit.

0x0 11y ago
Squashing a large feature branch for rebase has a downside that it loses some history and timeline context; if for example an api changed slightly in master in between branch development and branch squash, it can be very confusing trying to understand why the commit did XYZ thing looking at the squashed diff - while if you keep the branch through a merge, it's easy to see that XYZ was written with the older API in mind.
- Osiris 11y ago
  This is exactly why I prefer merge commits, so you can see how the development of the feature occurred and if there's a bug where/when/how it was introduced. I don't like erasing development history for the sake of a "clean" or "linear" history.

bsimpson 11y ago

A nice side-effect of having master===production is that your documentation on GitHub (or whatever web viewer you use) is always consistent with the deployed version. Otherwise, you could have users come to your repo looking for docs for the version they've installed, and instead get docs about APIs that aren't released yet.

sciurus 11y ago

What you describe for testing and merging is a needlessly manual process. Using e.g. Phabricator, I can make a local feature branch and when I think it's ready open a request for code review. My CI system can run tests. I can update my request in response to test failures or reviewers comments. When my code is ready, Phabricator can squash all my commits into one and rebase that onto master. If for whatever reason I did want to back out this code, there's a single commit to revert. This is unlikely though, since until a feature has proven itself in production it will be behind a feature flag. https://secure.phabricator.com/book/phabflavor/article/recommendations_on_branching/ https://secure.phabricator.com/book/phabflavor/article/recom...

Osiris 11y ago
That's a nice process. Currently, GitLab doesn't support running tests on a merge request in that sense, making a temporary merge of master and the branch and running the tests to confirm the merge will work. When/If GitLab + GitLab CI adds that ability, then yes, it would make that process a lot easier.
- sytse 11y ago
  We're discussing adding a 'test the merge' ability in 8.7 in https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 This will test the 'current merge' as soon as a commit is pushed, not the merge result when it happens.