forking and pull requesting about espressopp HOT 10 OPEN

Is there actually a reference page where guidelines are set?

from espressopp.

I support this question.

from espressopp.

My few pennies...
I am not sure that there is some official strategy. The fork-branch workflow seems to be a clean way to deal with git. (here is something on that: https://goo.gl/ZcGIWU)
From my perspective, I would suggest to work with it for every even small changes, if we make any exclusion then we end up with the same situation like now. The developer team is small so there is no point to really assign anyone, IMHO code review is not to validate the method from theoretical point of view (that should be proven via unit tests) but only to spot some strange structure that maybe someone knows how to write better/optimal/..
Moreover it would be useful to have master, hotfix and develop branches in the main repository to at least keep some stability with the code.
And indeed, having some guidelines would be very beneficial and also more extensive communication, e.g. suddenly some documentation format was introduced everywhere.

from espressopp.

@stuehn, @govarguz and I discussed that a couple of weeks ago.

I think the main advantage of the fork and merge-request strategy is that, you get testing before the code is in the master branch and that prevents having a broken master. In addition somebody else has a chance to look at the code and check for obscure parts. Of course currently we only have 5 tests, which makes the testing not very useful, but the infrastructure in cmake is there, so it isn't hard to add more test.

Concerning the branches, I would say that currently master is the development branch and we had a stable branch a long time ago, which nobody maintained and hence got deleted. We could do development on branches instead of forks, but that simply clutters up the main repository. And again it needs a maintainer, which deletes dead end branches.

from espressopp.

Once again I agree with @junghans on the fork and merge-request strategy. Moreover I saw that in the meanwhile of this issue a few not updated/maintained branches have been deleted.

from espressopp.

Ok, I do understand now that the idea behind is to check the code a bit and prevent broken master through testing. But weren't these compilation tests there also before forking/merge_request strategy?

Could we make a decision on communication of important changes (e.g. branching vs. pull_requesting)? Maybe we finally DO NEED a meeting for developers where external ones can participate through Skype/Video Conference? We can choose a reasonable time and frequency and discuss/explain/understand such changes without asking Torsten/Horacio/Christoph independently.

I think I am not the only person whom the whole "development" process seems to be VERY chaotic.

from espressopp.

Besides testing, you get code review and because 4 eyes are better than 2, it usually improves the code.

@niktre , maybe you should start a wiki page on the development process, so that we have something to talk about a the next development meeting.

from espressopp.

Hi all,

My earlier question was just to inquire about the status of things :-)

I like to develop on branches and find the tools that github offers very efficient. They have a page on the GitHub flow.

I tried to find a nice reference of developing practice and found a few interesting links. None are completely satisfactory though.

• https://github.com/ViennaRSS/vienna-rss/wiki/Good-manners-with-Git
• http://blog.ploeh.dk/2015/01/15/10-tips-for-better-pull-requests/
• https://blog.rainforestqa.com/2014-05-28-version-control-best-practices/
• https://github.com/ipython/ipython/wiki/Dev:-GitHub-workflow

A short list could be:

• Do not leave commented code. This does not mean "do not use comments" indeed, simply not actual programming in C++ or Python commented.
• Do not merge your own PR.
• Keep commits focused on a well-defined task.

Given ESPResSo++ relatively small team things do not have to be set in stone.

from espressopp.

Should we move this question to wiki?

from espressopp.

As @acfogarty mentioned, we discuss here what conditions have to be met by pull requests. As soon as the final version is clear, I'll start to put it on wiki for generations of the lucky ones who will start with ESPResSo++ in the future. And for us as a reminder.

So, what should be inside pull requests? @pdebuyl has already sketched the mains:

1. Code that is more or less focused on a well-defined task
2. Take care of commented code. Sometimes we need it, but not always.
3. Don't forget about comments to the code explaining shortly what's happening (has nothing to do with item 2.)
4. Unittests
5. Documentation
6. As soon as pull request is done add it to the Roadmap in "Projects" board

Of course, if the pull request is a bug fix, the above list is crap. In this case it just has to fix some issue.

from espressopp.