More Community-driven Development

[Cornstalks]

I'm not a big fan of patches, but that would be my second choice. One can clone the repository, work on it, and then export all his modifications to one or more patch files, that I can then apply quite easily. I don't know if the corresponding commits will be marked as being from him or from me, though.

If the patches were created with git format-patch, then they will have him as the author and you as the committer. If they just send you a diff instead of a proper patch, then author information is lost and you have to specify it using --author when committing. It looks like this when done.

I prefer to look at patches/diffs, and I've learned that in a pull request on Github, you can click the "Files Changed" tab to get a full diff of the merge request. I think (but I haven't tested so I'm not 100% sure) that if a user adds commits to a pull request (perhaps cleaning up code Laurent points out), then the "Files Changed" tab shows the diff for the pull request in its entirety; that is, the diff also has the new commits applied.

We can also comment on individual lines of a pull request, which gives more or less the same ability as picking appart a patch on a mailing list, as I understand it.

Anyway, I'm looking forward to contributing to SFML someday (probably summer, again).

Edit: accidentally said contributor instead of committer

[Laurent]

but it will probably save Laurent's time overall.

Yep, that was my main concern.

It helps to parallelize work, and to avoid bloating my todo-list: instead of me doing several small tasks, it's dispatched among many contributors who can work on their side. So it actually saves time

[Nexus]

I find this part questionable, because it seems to me we're talking about:
- dropping a comment on github
- merge the pull request,  open your development environment, make the changes yourself and commit them.

I spoke about tiny imperfections, not big issues that have to be fixed in order to make the pull request merge-ready.

Personally, I find it far more annoying to write something like

Line 25: Use if (x) instead of if(x)
Line 35: Comment should be worded "// After closing the window" instead of "//window close"
Line 60: Break the line before the third function argument
Line 80: Call template argument T, not Type
Line 92-98: Group the draw() calls together, and clear() and display() with an empty line in between
Line 111: File ends with newline

instead of just fixing those points myself. As soon as there are a lot of such issues (which is likely in a bigger feature request or bugfix), one wastes a terrible amount of time at formulating corrections. If a point is overlooked by the pull requester, I have to write another comment. Each time, I have to look at the code (or at least the diff) to check if everything was implemented correctly. I always have to wait for the requester to adapt my suggestions. If he is inactive, the whole progress stalls. And I still don't see whitespace inconsistences because of web interface limitations.

To perform tiny corrections, I take into account a massive communication overhead.

It depends of course on how picky you are, but I prefer a consistent and clearly understandable code style. And yes, the status quo on GitHub doesn't make it very simple to edit pull requests, but I don't know of any better way than to create a new local branch.

[Laurent]

I don't think that you can spot all these tiny issues in a big pull request. You'll rather notice consistent differences between your style and the contributor's style, which can be explained in a short comment.

For the small inconsistencies that you're mentionning, I think they'll rather be spotted later, when looking at or working on the code again, and thus they can be corrected as easily.

Anyway, for now it's not a blocking point for me, so let's try this way. If one day it becomes a pain to review big pull requests, I'll think about another strategy

[Tank]

I totally agree to Nexus. Personally I don't really like how GitHub handles merging at all. It hides all the power Git provides by doing a 3-way merge all the time, sometimes resulting in very ugly graphs that could have been done better.

When I had projects at GitHub and got a pull request, I added the user's repository to my local remotes, "git fetch --all" and took a look at the commits in question. You can easily make some adjustments, commit them and either merge or rebase them onto your master/current branch.

If you're doing it like Laurent, i.e. merging when everything's 100% sane, then the 1-click merge might be all good. Personally I also like to take a contribution and use it as a base for a "corrected" version or similar.

[Cornstalks]

I've been thinking about this a bit, and one thing that I think would really help the community to get involved is to get their names in the commit history of SFML. There are tons of pull requests that seem to get the response "I used your changes as a base but didn't like the way you did things so I rewrote it myself."

I think it's fine to be nitpicky, but I honestly think it would help encourage and motivate others to contribute to SFML if there names actually showed up in the commit history. If you could tell them specifically what needed to change so they could make the necessary changes in order for the patch to be "good enough," I think it would help people better understand the SFML coding style (so their next patch is more likely to need less changes) and it would act as a good boost of motivation to work on another patch.

Thoughts?

[Nexus]

Do you really think the main motivation of contributing is to appear in the commit lists? I don't know if it's good to go in that direction, it might lead to people saying "look, I am part of the SFML development" or competitions between users concerning who contributes most, and a lot of trivial commits of which the merge overhead outweighs the feature -- in other words, people who only contribute for the sake of being visible in the commit list. Also, the repository owner shouldn't be under the pressure of applying a commit, because not doing so or implementing it himself (which would be faster in some cases) would appear as "unfair compared with others".

Of course, this is a pessimistic view, but eventually there will be a few people acting like this. In my opinion the motivation should generally be a different one, namely progress. If one really wants to push SFML (and even if it's only to have more features in one's own project), it's probably not too relevant who made which commit, especially not for smaller contributions. If it is (and I agree that notable contributions deserve to be credited), Laurent can still mention the name in a commit message, if that's easier to handle.

And I still find the points in my last post important, but of course it's up to Laurent how this is handled. But you can be sure, even if the SFML conventions are listed again and again, many people will not follow them exactly. In the case of smaller contributions, constantly rejecting commits and hinting to the mistakes takes up much more time than implementing things oneself. You see it in this forum, Laurent made a detailed thread explaining the forum rules and how to post a question meaningfully, yet every beginner ignores it. Sadly, sometimes even users with many posts seem not to care about it.

I'm not saying that people shouldn't be mentioned for their contributions -- but it does not justify a lot of additional effort, such as nitpicking trivial commits. So I would certainly not come up with something like a guideline that enforces the attribution of commits.

[Cornstalks]

Do you really think the main motivation of contributing is to appear in the commit lists?

I don't think it's a primary motivation (nor should be), but it's a form of positive feedback. It's encouraging to know you've made a positive impact.

I don't know if it's good to go in that direction, it might lead to people saying "look, I am part of the SFML development"

But aren't they? They may not be a primary/core developer or a maintainer, but they can certainly claim to have helped and contributed.

or competitions between users concerning who contributes most, and a lot of trivial commits of which the merge overhead outweighs the feature -- in other words, people who only contribute for the sake of being visible in the commit list.

I've never had such "dick measuring" (sorry) be an issue in the open source projects I've followed and contributed to. I really don't think that would be an issue.

Also, the repository owner shouldn't be under the pressure of applying a commit, because not doing so or implementing it himself (which would be faster in some cases) would appear as "unfair compared with others".

Oh sure, trivial things are easier to reject, IMO.

Of course, this is a pessimistic view, but eventually there will be a few people acting like this.

I doubt it'll be problematic.

In my opinion the motivation should generally be a different one, namely progress.

In case it wasn't clear, I don't think your name in the commit history should be your real motivation. Progress should absolutely be your primary motivation. But positive recognition can certainly be encouraging and help strengthen your motivation. But even if someone was purely motivated by recognition... well as long as SFML progresses, I don't really care.

And I still find the points in my last post important, but of course it's up to Laurent how this is handled. But you can be sure, even if the SFML conventions are listed again and again, many people will not follow them exactly.

True, but if a clear style/conventions guide was available, all it would take is a one sentence "Make sure you're adhering to this style guide; rejected until fixed." and then you don't have to say any more about the trivial stuff. Either they'll fix it, or it will stay rejected.

In the case of smaller contributions, constantly rejecting commits and hinting to the mistakes takes up much more time than implementing things oneself.

If you've got a clear style guide, again, you can just link to it and say it's rejected until fixed. Make them figure it out. It'll weed out the serious from the not serious.

[Nexus]

I doubt it'll be problematic.

Yes, as said it's pessimistic. I just wanted to note that there might also be negative effects, how this looks with this specific project and its users is a different question.

True, but if a clear style/conventions guide was available, all it would take is a one sentence "Make sure you're adhering to this style guide; rejected until fixed." and then you don't have to say any more about the trivial stuff. Either they'll fix it, or it will stay rejected.

Ok, if it is realized this strictly, it might work. And in the long term, being strict is probably the only way to go.

[binary1248]

Progress above all. I can't care less if my name shows up in the commit history or not. Knowing that I spent 2 months working on a fix that had a positive impact on SFML is enough satisfaction for me. If anyone wants proof that I really did contribute, I would just point them to the related forum post (if one exists) or the issue on the tracker. At least Laurent has the decency to mention that he applied a modified version of your commit and didn't just silently close the issue pretending you didn't do anything useful.

If someone really insists that they only recognize your contribution through the commit history although those alternatives exist, that's one more candidate for your blacklist of people you shouldn't bother explaining things to.

Progress => Self Satisfaction => Win

One day SFML will be the most used multimedia library in use and I will be able to honestly state that I was part of that progress, a long time ago in its little corner of the internet . Until then, happy debugging and coding...

[Laurent]

Problem is time, as usual.

I'd like to have the time to write a complete style guide for contributors, but I don't. Sometimes I take the time to tell people what to change so that their pull request can be merged, sometimes I don't because I really have no time. I know, it's so sad

[Cornstalks]

Problem is time, as usual.

I'd like to have the time to write a complete style guide for contributors, but I don't. Sometimes I take the time to tell people what to change so that their pull request can be merged, sometimes I don't because I really have no time. I know, it's so sad

Would it help if we (the community) wrote a style guide (which you would correct and revise when you had time), and then once you're satisfied with it then it's done? Once it's all written, it should be pretty easy to integrate it into the website (of course, this wouldn't be done instantly, but I'm trying to find ways to help out).

I certainly understand the issues with time.

[Laurent]

Yes, that would help a lot

[Cornstalks]

I created a style guide page on the wiki. It would be awesome to have the whole community contribute to it! I'm just reading SFML's source code, finding patterns and documenting them. Right now I'm trying to create a general skeleton of the style guide, and after that it's just a matter of filling in the details.

[Laurent]

Awesome, thanks