We have vaguely been using the git flow workflow for several years without actually following its tenets. Branching off of master rather than develop when you have a critical bugfix is one of the tenets though. In general, I agree about not cherry-picking, but I did in this one case to fix the tests.
But one is not formally a revert; it is just a change that yields the same lines as the revert does.
Agreed. I don’t think there is a formal “Revert” commit though, is there? AFAIK the
revert command just constructs the inverse commit for you.
git revert is a bit different because you cannot then merge the branch again (not that we want to in this situation).
I can’t find any doc to that effect - do you have a link? Not that it matters for this really, just curious. git provides infinite content to learn about, it seems.
Anyway would you be okay with just creating a new tag for v2.17.1 that has a commit that doesn’t live anywhere else? And then the weird history is confined to a small short-lived branch and not the main timelines of master or develop.
The unit tests and test-headers pass after merging develop into bugfix/revert_string with a one-line change, so I think that is going to be fine.
Hm, not seeing it in there but I’ll look further.
What’s your objection to a v2.17.1 tag with an orphaned commit?
Also what commit is 35f6f86a4a? I don’t seem to have that hash locally…?
It may be 34df1bcce31a45ebd83abd38718c4b940eb44adb for you.
If we release off an orphaned commit and someone checks out master, they have the slow version. All interfaces should be using the master branch.
That sucks. At least that problem is short-lived, though (until our next release), and people who need pre-C++11 have an option still.
This is getting pretty complicated and I’m pretty worried about having a diverged master and develop like that with multiple commits that are duplicated (we’d actually need at least 3, there’s another one needed to make the tests pass on g+±4.9). Spot checking the conflicts looks fine but I would definitely not feel comfortable extrapolating when we’re fucking with the history like this. I’ve been burned before by automatic merge conflict resolution.
Is this worth the trouble to do this as a C++99 release at all? It’s also a bad time for me personally to be doing backflips in git - I really need to work on this SBC paper.
Although cherry-picking is a bad idea for development with git, it isn’t going to confuse git. If there is 34df1bcce31a45ebd83abd38718c4b940eb44adb in one tree and 34df1bcce31a45ebd83abd38718c4b940eb44adb in another, it is going to look the same unless there are subsequent commits in each tree that modify those same lines differently.
I think I had everything working but had not run the distribution tests.
That’s fair (though the hashes change). But tons of those files were actually also modified in develop too. Are you sure taking develop’s changes always works?
I did the unit tests (but not the distribution tests) after auto-merging develop into bugfix/revert_string and it was fine after I fixed one file.
But I think it is safe to merge bugfix/revert_string into master and release 2.17.1. If there are problems in merging develop into master for 2.18.0, then future us will have time to identify and fix them.
Just to clarify: I do not have any issue in Stan going C++11 dependent. Luckily our cluster got a g++ 6.3 installation since recently.
A motivation for a non-C++11 release could be to make those RHEL6 users happy - and I don’t know if the gcc 4.8.1 on RHEL7 can deal with the C++11, but RHEL7 users always have the choice to install g++ 6.3.
So maybe there is not such a big need for a non-C++11 need and we better do a 2.18?
I prefer 2.18.0 unless someone wants to speak up about their use case relying on an older compiler? I think we do a lot of things for hypothetical users and we should stop speculating and ideally send out a survey or something if we’re going out of our way to support them, just to make sure the work is for actual people and use-cases.
If we do have a use-case for g++4.6 still, I’d like to dive into why the interfaces are referencing
master - what does this mean? Shouldn’t they be released referencing a specific tag or commit? If not, we should start that.
We never even announced that 2.17.x would be the last Stan release that did not have C++14 features.
We know that servers often tend to have g+±4.8 or lower installed and that support contracts or conservative system administrators prevent installation of newer toolchains.
Conversely, I think you are worried about a non-problem of merging develop into master. One of the core tenets of Gitflow — which we have ostensibly but not really been following for four years or so — is that you branch off master when there is a critical bug and release after merging the branch that fixes it into both master and develop. Your position seems to be “that might not work” in ways that are unfixable several months from now. How is our situation different from that of tens of thousands of other software projects that use Gitflow?
We didn’t branch off master to do the bugfix. If we had done that I’d be on board.
Instead we’re talking about cherry picking and later merging the cherry picked commits in, which people recommend against.
If it helps, i can offer to check if rhel7 can build whatever we want to release…using the default 4.8 system compiler. I just would need instructions what to do.
I did branch of master and the recipes I posted branched off master
git checkout bugfix/revert git log --oneline | head
35f6f86a4a Tests corrected now. fe171373c0 Revert "Merge pull request #584 from stan-dev/feature/issue-460-const-char-star" 4b1a10bc87 release/v2.17.0: updating version numbers 5553b45afb Merge pull request #611 from stan-dev/build/new-boost-new-flag c6ede0ab4d Fixes issue discussed in stan-dev/stan#2384 to close stan-dev/stan#601 199c320143 Bugfix/599 upgrade boost 1.64.0 (#601) 2457932d49 Merge pull request #610 from stan-dev/build/travis-split 5339cbe3e0 Split out travis tests more - timing is unpredictable 908d528c0b Merge pull request #577 from bbbales2/feature/issue-481-append-array-vectors-of-vectors 82ca96a443 Merge pull request #606 from stan-dev/build/travis-test-mix