Re: [Numpy-discussion] proposal: new commit guidelines for backportable bugfixes
On Sat, Jul 19, 2014 at 12:44 AM, Pauli Virtanen p...@iki.fi wrote: 18.07.2014 23:53, Julian Taylor kirjoitti: On 18.07.2014 19:47, Pauli Virtanen wrote: [clip] The other well-known alternative to bugfixes is to first commit it in the earliest maintenance branch where you want to have it, and then merge that branch forward to the newer maintenance branches, and finally into master. wouldn't that still require basing bugfixes onto the point before the master and maintenance branch diverged? otherwise a merge from maintenance to master would include the commits that are only part of the maintenance branch (release commits, regression fixes etc.) If I understand correctly, the idea is to manually revert the changes that don't belong in, which needs to be only done once for each, as the merge logic should deal with it in all subsequent merges. I think there are in practice not so many commits that you want to have only in the release branch. Version number bumping is one (and easily addressed by a follow-up commit in master that bumps it again) --- what else? The bugfix-in-release-and-forward-port-to-master seems to be the recommended practice for Mercurial: http://mercurial.selenic.com/wiki/StandardBranching https://docs.python.org/devguide/committing.html I think there are also git guides that recommend using it. The option of basing commits on last merge base is probably not really feasible with Mercurial (I haven't seen git guides that propose it either). basing bugfixes on maintenance does allow cherry picking into master as you don't care too much about backward mergeability here, but you still lose a good git log and git branch --contains to check which bugfix is in which branch. I don't disagree with this. Cherry picking is OK, but only as long as the number of commits is not too large This should be the case most of the time I think. It looks like we've started backporting more and more though, even things like minor doc fixes. The maintenance overhead would be much lower if we would stick to only backporting important bug fixes. Any strategy chosen is fine with me, but I would like to see considered how this affects the number of PRs and the complexity for occasional contributors. Those contributors can't really judge what's backportable and don't want to deal with rebasing. So the new strategy would be something like: 1. bugfix PR sent to master by contributor 2. maintainer decides it's backportable, so after review he doesn't merge PR but rebases it and sends a second PR. First one, with review content, is closed not merged. 3. merge PR into maintenance branch. 4. send third PR to merge back or forward port the fix to master, and merge that. (or some variation with merge bases which is even more involved) Compare to what we did a while ago for numpy and still do for scipy: 1. all PRs are sent to master 2. hit green button after review 3. bugfix is cherry-picked and pushed directly to the maintenance branch The downside of the second strategy is indeed the occasional extra merge conflict, but having 3x less PRs, 2x less merge commits and a less confusing process for occasional contributors could well be worth dealing with that merge conflict. Cheers, Ralf and you use a tool (e.g. my git-cherry-tree) that tries to check which patches are in and which not. Pauli ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] proposal: new commit guidelines for backportable bugfixes
19.07.2014 11:04, Ralf Gommers kirjoitti: [clip] 1. bugfix PR sent to master by contributor 2. maintainer decides it's backportable, so after review he doesn't merge PR but rebases it and sends a second PR. First one, with review content, is closed not merged. 3. merge PR into maintenance branch. 4. send third PR to merge back or forward port the fix to master, and merge that. (or some variation with merge bases which is even more involved) The maintainer can just rebase on merge base, and then merge and push it via git as usual, without having to deal with Github. If the pull request happens to be already based on an OK merge base, it can be merged via Github directly to master. The only thing maintainer gains from sending additional pull request via Github is that the code gets run by Travis-CI. However, the tests will also run automatically after pushing the merge commits, so test failures can be caught (although after the fact). This is also the case for directly pushed cherry-picked commits. -- Pauli Virtanen ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] proposal: new commit guidelines for backportable bugfixes
On Sat, Jul 19, 2014 at 12:29 PM, Pauli Virtanen p...@iki.fi wrote: 19.07.2014 11:04, Ralf Gommers kirjoitti: [clip] 1. bugfix PR sent to master by contributor 2. maintainer decides it's backportable, so after review he doesn't merge PR but rebases it and sends a second PR. First one, with review content, is closed not merged. 3. merge PR into maintenance branch. 4. send third PR to merge back or forward port the fix to master, and merge that. (or some variation with merge bases which is even more involved) The maintainer can just rebase on merge base, and then merge and push it via git as usual, without having to deal with Github. I agree, but note that that's not what's happening in the numpy repo at the moment and that Julian (and maybe Chuck as well?) is explicitly against any direct pushes. So the 3x more PRs between what the process used to be and what Julian proposes is not unrealistic. Maybe still worth it, but it's a trade-off (example: I used to use gitk --all, but it's a spaghetti now). Ralf If the pull request happens to be already based on an OK merge base, it can be merged via Github directly to master. The only thing maintainer gains from sending additional pull request via Github is that the code gets run by Travis-CI. However, the tests will also run automatically after pushing the merge commits, so test failures can be caught (although after the fact). This is also the case for directly pushed cherry-picked commits. -- Pauli Virtanen ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] proposal: new commit guidelines for backportable bugfixes
On 19.07.2014 13:04, Ralf Gommers wrote: On Sat, Jul 19, 2014 at 12:29 PM, Pauli Virtanen p...@iki.fi mailto:p...@iki.fi wrote: 19.07.2014 11:04, Ralf Gommers kirjoitti: [clip] 1. bugfix PR sent to master by contributor 2. maintainer decides it's backportable, so after review he doesn't merge PR but rebases it and sends a second PR. First one, with review content, is closed not merged. 3. merge PR into maintenance branch. 4. send third PR to merge back or forward port the fix to master, and merge that. (or some variation with merge bases which is even more involved) The maintainer can just rebase on merge base, and then merge and push it via git as usual, without having to deal with Github. I agree, but note that that's not what's happening in the numpy repo at the moment and that Julian (and maybe Chuck as well?) is explicitly against any direct pushes. So the 3x more PRs between what the process used to be and what Julian proposes is not unrealistic. It is what is happening at the numpy repo. We are never directly pushing unreviewed changes, we always have at least one PR. We only directly push changes that have been approved to be applied two more than one branch. With the method I propose there are not any more PRs. You have the main PR targeted to master and the bugfix PR targeted to the maintenance branch, it was the same before except the bugfix PR was a cherry pick instead of a merge. When directly pushing the second merge we even cut one PR from the process. E.g. I pushed Pauls PR #4882 directly to 1.9 without asking him to create a new PR but as far as git is concerned there is no difference, it as still two merges. We could always ask for a new PR for the branch merge to see travis results before the merge. E.g. #4877 and #4891 same branch two PRs two merges. I don't think that should be currently required as master and 1.9 are almost identical and there is little value in seeing travis results for the second merge before doing the merge. But when the branches diverge more the two PRs should probably be preferred to avoid having broken commits on the branches that make bisecting harder. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] proposal: new commit guidelines for backportable bugfixes
On Sat, Jul 19, 2014 at 1:26 PM, Julian Taylor jtaylor.deb...@googlemail.com wrote: On 19.07.2014 13:04, Ralf Gommers wrote: On Sat, Jul 19, 2014 at 12:29 PM, Pauli Virtanen p...@iki.fi mailto:p...@iki.fi wrote: 19.07.2014 11:04, Ralf Gommers kirjoitti: [clip] 1. bugfix PR sent to master by contributor 2. maintainer decides it's backportable, so after review he doesn't merge PR but rebases it and sends a second PR. First one, with review content, is closed not merged. 3. merge PR into maintenance branch. 4. send third PR to merge back or forward port the fix to master, and merge that. (or some variation with merge bases which is even more involved) The maintainer can just rebase on merge base, and then merge and push it via git as usual, without having to deal with Github. I agree, but note that that's not what's happening in the numpy repo at the moment and that Julian (and maybe Chuck as well?) is explicitly against any direct pushes. So the 3x more PRs between what the process used to be and what Julian proposes is not unrealistic. It is what is happening at the numpy repo. We are never directly pushing unreviewed changes, we always have at least one PR. We only directly push changes that have been approved to be applied two more than one branch. OK never mind then. I was pretty sure you said you were against this, and I see a lot of PRs for simple backports in 1.8.x and 1.9.x. If you now say it's fine (or even preferred) to push directly, my worry about multiple PRs isn't relevant anymore. Ralf With the method I propose there are not any more PRs. You have the main PR targeted to master and the bugfix PR targeted to the maintenance branch, it was the same before except the bugfix PR was a cherry pick instead of a merge. When directly pushing the second merge we even cut one PR from the process. E.g. I pushed Pauls PR #4882 directly to 1.9 without asking him to create a new PR but as far as git is concerned there is no difference, it as still two merges. We could always ask for a new PR for the branch merge to see travis results before the merge. E.g. #4877 and #4891 same branch two PRs two merges. I don't think that should be currently required as master and 1.9 are almost identical and there is little value in seeing travis results for the second merge before doing the merge. But when the branches diverge more the two PRs should probably be preferred to avoid having broken commits on the branches that make bisecting harder. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] BLAS / LAPACK / MKL cannot be found?
Okay I figured out how to do it, in case someone finds this message later. You need to enter this specific section for the MKL implementation of BLAS and LAPACK, and it will find it! #https://software.intel.com/en-us/articles/numpyscipy-with-intel-mkl [mkl] library_dirs = /wgdisk/omega2dev2/env/EL5/intel/composer_xe_2013.0.079/mkl/lib/intel64 include_dirs = /wgdisk/omega2dev2/env/EL5/intel/composer_xe_2013.0.079/mkl/include mkl_libs = mkl_rt lapack_libs = Note that no libs are given for lapack_libs. This is not a failed copy-paste :) Hopefully this will help someone! ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
