Joseph wrote:
Christoph wrote:
• (a3) Unnecessary merges are avoided. Some people like a policy where each set of patches is merged explicitly. I think this is bogus. If a change can be put as a single commit (as it is the case very often), it should be best applied as a single commit on top of the current history. Unnecessary merges obscure the project history and make relations between features less evident. This makes understanding the history much more complicated, and things like bi-secting for a bug work less well.I would like to contest the point that a set of commits should be squashed into a single one.
I didn’t say that everything should be squashed to one commit. I just said that I think that the policy “explicit merges always” is not a good one. I think it’s preferable to rebase a smallish isolated commit instead of merging it (this creates two commits and a more complex history graph). Gitlab actually has a feature to try to rebase a merge request and to apply it to the top if it rebases cleanly, but only in the “enterprise version”…
For more complicated topic branches that consist of several commits it may be even clearer to merge them, as this groups the commits together and gives them a common name. This is certainly true for feature branches with a convoluted history. For feature branches that consist of a clean series of commits, I think it doesn’t matter so much whether they are merged or applied at the top.
While this is a very admirable goal, I think that what is more important is having a standard workflow that everyone can use. This lowers the barrier to entry to new contributors. Currently the "you can contribute however you like!" strategy is very nice if somebody already knows how to contribute to an open-source project, but for a newcomer it is impossible to get off the ground without some standard way of doing things. Of course the Standard Way should probably also be the way the maintainers typically work, as they will be the ones doing the review + making the most contributions.
The ability to participate in discussions by replying to email messages is already very nice. Is it possible to make the emails sent by gitlab pure text (no HTML)?
Ideally, I think, it would be great if all the discussions could be mirrored to a mailing list (that could be a dedicated list, or, if the volume is acceptable, kwant-devel). This would provide a convenient way to watch for new things without having to poll the website regularly.
The above improvements to the workflow can all be provided by either of the tools proposed, however it is useful to have them preserved for posterity here on the mailing list.
Well, I think even if we setup a gitlab instance, there will be still a barrier for people who live exclusively on github. So it may be a good idea to have a github mirror of Kwant as well. If we disable issues, we do not force anyone to use github, so if there is some volunteer to look out for PRs on github, that might be useful.
Before we continue with evaluating various tools, let’s first discuss what we would like to have. I’m looking forward to your comments.Essentially the features that you raised above. I think a transparent way to do code review is important. Neither Phabricator nor Gitlab is perfect in this respect. Phabricator only stores the diffs, not the commits that led to a diff. This means that if a maintainer merges some accepted changes then these are necessarily in one big commit. This can be annoying if someone has implemented some feature with nontrivial changes where it makes sense to keep the commits separate. In Gitlab the problem is that there is no history of a patchset kept when dealing with merge requests[1] (although I have an idea as to how this could be fixed).
While I like its more proper support for iterative code review, phabricator seems to be quite intrusive in various ways (It insists on adding cruft to all commit messages, for example).
If I undertand corretly, Gitlab, on the other hand, can be used as a mere git respository hosting server (like gitolite), so it does not force us to use any of its extra features.
Christoph
smime.p7s
Description: S/MIME cryptographic signature
