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

Stable branch for releases #796

Closed
xzyfer opened this issue Jan 1, 2015 · 15 comments
Closed

Stable branch for releases #796

xzyfer opened this issue Jan 1, 2015 · 15 comments

Comments

@xzyfer
Copy link
Contributor

xzyfer commented Jan 1, 2015

This applies mostly to @hcatlin, @mgreter and @akhleung but @am11 and @QuLogic may have feelings.

What

I'd like to propose we create a stable branch which will reflect the state of the latest stable release and which we would be doing releases from.

Why

As @hcatlin discovered during SassConf '14 the number of people relying on Libsass is significant. The rise in adoption means providing stability is more important than ever.

We've seen a boom in the pace of development recently. This has lead to meatier releases (3.1.0 has ~22 new features, and ~25 bug fixes). The bigger the release the more prone we are to introducing regressions (instability).

Due to the nature of how Libsass is largely used (via upstream vendors i.e. node-sass) simply doing smaller, faster releases isn't necessarily viable since there is a delay between our releases and user feedback.

Also the upcoming features in 3.2, 3.3 and 3.4 are significant, and touch a lot of code points. They should be treated as significant releases.

The problem

We're on the cusp of the 3.1 release!

We're writing significant features and bug fixes faster than ever. This fast pace means we already have a couple significant patches lined up for the first patch release, and even new features for the 3.2 release.

Assuming there will inevitably be bugs in the 3.1.0, and we don't want to ship a major feature in order to get a small patch out. We end up unfortunately leaving 3.2 features in branches to get stagnant as we do the smaller patch releases. To be clear I'm saying this is good, and the right thing to do for stability

A problem I'm already running into now is in order implement @at-root I need my work on #790, and branching off branch is just going to lead to mess and pain.

The solution

I'm proposing we create a stable branch that mirrors the current release. Generally speaking we still work on master.

In fact this is what the Ruby sass team does!

Dealing with patches

Patches would be created, and PR'd against stable. We'd merge the PR for the upcoming patch release and do the release. We could then either cherry-pick those patches (my preference) or merge stable into master (this makes for a messy git history IMHO).

Dealing with major features

We could now merge all major feature branches directly into master at will.

Dealing with major releases

Since master will be up to date with stable it would just be a simple merge of master into stable. 💥

Thoughts and feelings?

@QuLogic
Copy link
Contributor

QuLogic commented Jan 1, 2015

We could then either cherry-pick those patches (my preference) or merge stable into master (this makes for a messy git history IMHO).

Contrarily, I think your preference is messier here. It's simpler and 'DRY'er to have only one commit for a bugfix. Since the stable branch has only bugfixes, it should be a pretty simple merge in general. There's no need to ask "did we include that bug fix from last year in the next major release?" because all you'd have to do is ensure that stable is merged. Built-in tools like git branch --contains abc1234 will list both stable and master and people won't have to go looking for the cherry-picked version. I'm sure there are other examples...

Everything else makes sense in general.

@am11
Copy link
Contributor

am11 commented Jan 1, 2015

One way to slice it is adding nightly branch instead of the stable. The nightly would always be the bleeding edge branch and master would simply be the one pointing to the latest stable. Or even more elaborated version would be:

  • master: pointing to latest stable tagged release.
  • nightly: pointing to latest preview tagged release.
  • dev: pointing to bleeding edge commit.

So at any point in time, master || nighlty would be sync'd with dev (depending on whether nightly was released most recently or stable).

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 1, 2015

@QuLogic

It's simpler and 'DRY'er to have only one commit for a bugfix. Since the stable branch has only bugfixes, it should be a pretty simple merge in general.

I completely agree. My concern is the need to merge stable -> master and master -> stable. Generally speaking it's cleaner, and less error prone to merge in one direction.

@am11 I don't know that nightlies make a sense, but I'm happy to be convinced otherwise. AFAIK master should always be a passing build so it would essentially be the nightly branch.

@QuLogic
Copy link
Contributor

QuLogic commented Jan 2, 2015

I completely agree. My concern is the need to merge stable -> master and master -> stable. Generally speaking it's cleaner, and less error prone to merge in one direction.

Ideally, there should be only one time that there's a merge from master -> stable: after a minor release. And since you've just made a release, you've also just merged stable -> master, so the master -> stable operation is just a fast-forward.

Of course, sometimes when you're working on a feature, you find yourself fixing a bug and you might need to cherry-pick that, but I'd say try and avoid it.

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 2, 2015

Yeah either way works. I'm simply not a fan of merge commits (on busy branches). They increase the signal to noise ratio and make things like bisects (and other git forensics) a little more complicated than need be.

I'm happy to try either approach. An alternative would be to have shorter lived branches per release i.e. 3.1.1 then merge that branch into master for the release.

Sass-spec makes things a littler tricker because once a spec has been activated all dev branches too have the patch/feature for that spec to pass builds.

@am11
Copy link
Contributor

am11 commented Jan 2, 2015

@xzyfer, on a related note, I think it would make sense to delete the exiting (unused/stale) branches https://github.com/sass/libsass/branches (and start fresh with this strategy). :)

@KittyGiraudel
Copy link

Adding to the discussion: on SassDoc, we have master as our stable branch. It reflects the latest stable version of the software. And we have develop which is the branch we work on for the next version.

For a bug fix, we pull a branch from master, fix the bug, then PR the new branch in master. Once reviewed and merged, we bump a patch.

When working on next version, we pull a branch from develop, work on the feature, then PR the new branch in develop. At some point it gets reviewed and merged. Once we have everything we want in our develop branch to release the next version, we make develop the new master and restart over.

This process is heavily inspired by git flow and works pretty well I must say.

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 3, 2015

@am11 I'm all for killing off old branches.

@hugogiraudel yeah that's pretty much the same thing. We can change the default branch to stable.

@mgreter
Copy link
Contributor

mgreter commented Jan 3, 2015

So here my 2 cents on this:

I guess you're all talking about http://techblog.md-systems.ch/tutorial_howto/git-branching-model !?
That's the model I am used to work with, but IMO it's not usefull if we do not have any "stable" definitions beside having it to pass the spec test suite (which seems to be a moving target anyway)! So IMO before we do not have any goals that define "stable", the whole exercise is pretty pointless!

IMO we should not yet adapt such a workflow, since it will increase burdens to introduce new features! As we already have a pretty good coverage with CI, this does not seem to bring much benefit! And since we are pretty careful to commit stuff directly on master, I feel the current way is sufficient ATM.

IMHO the basic question boils down to: "do we want to preserve the current state of the code base?". Which I would say no! We still want to improve it significantly, which needs us to be able to push and include major changes to to current code. One of this points would be source-maps, which need to change dramatically to work correctly. Stuff like this will add new regressions, but I don't see any other way to get it to the state we want it to be in!

Edit: What I basically want to say: I dont see any reason to have a master behind develop, Just for the sake of being sure we have a stable master (again, what means "stable"). IMO currently we ensure "stable" with CI! And since PRs run against the CI service, I don't see any need so far to split up our developement in several branches. If you want/need a specific version, take a tag or a certain commit id!

Final Note: If we want to talk about stable libsass, we should at least first provide a stable ABI!

As a side note: I'm also for killing stale branches, but most seem to be by @akhleung, and I don't want to purge stuff that might be valuable (most branches seem to have at least one unique commit).

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 3, 2015

I guess I can boil down my concern to the following scenario.

I'd done a major feature for bubbling media queries. Now that 3.1 is out I'd like to merge to so I can build on that work for at-root. However if when node-sass 2.0 is released, and a simple one line bug is patched, I don't want that massive feature being rolled out in say 3.1.1.

Doing so risks introducing totally new regressions. One step forward and two steps back.

@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 12, 2015

Considering the progress we've made on the 3.2 and 3.3 milestones this may be a moot topic for the time being. Closing.

@xzyfer
Copy link
Contributor Author

xzyfer commented Apr 15, 2015

We've recently seem demand for some way to back port patches to older versions. This is currently difficult to do.

#850 (comment)
sass/node-sass#858 (comment)

Alternatively we could aim to do smaller incremental release which minimise the likely hood and severity of regression.

@saper
Copy link
Member

saper commented Apr 16, 2015

https://github.com/saper/libsass/tree/fix/3.1/850 contains the fix on the 3.1.0 branch.

Seems like one cannot create pull requests to tags or releases, only to branches.

@mgreter
Copy link
Contributor

mgreter commented Jan 22, 2016

@xzyfer IMO such branches only make sense if we have some kind of long term support for a specific version. And IMO we are far away to even consider something like that since even ruby sass does not have such a thing. We also have a pretty good CI now, so I also don't see a reason to split develop and master, since we do not have and cannot lift and support any integration testing stage to verify the transition of the code between develop and master (beside CI that we already have).

We have one master which has to pass CI, which means that every commit that returns green light on CI could be a stable release. I myself do some more extended tests with perl-libsass to verify some stuff on the C-API which isn't yet covered by our CI, once we get ready to tag a release. So that is IMO the next pain point after we have now error spec testing and can also test different precision options (in case you missed that). Testing the C-API via CI would be the next big goal here ...

So I don't see a reason why we would need additional branches beside developing in feature branches and bringing the changes into master via PRs, which are CI tested!? As I initially said, this would IMO only change if we would need to support LTS releases.

So in my opinion this issue could be closed!

@xzyfer xzyfer closed this as completed Jan 22, 2016
@xzyfer
Copy link
Contributor Author

xzyfer commented Jan 22, 2016

👍

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

No branches or pull requests

6 participants