Since my point doesn't seem to have come across, I'll try again ;-)
On 02/18/2011 01:13 AM, Jeremy Mikola wrote:
Couple ideas on this:
We should avoid pull requests with many commits (one or a few should be
sufficient).
Many small commits are good, few big commits are bad because it becomes
difficult to tell precisely what was changed for which reasons.
On 02/18/2011 01:13 AM, Jeremy Mikola wrote:
Many commits in a single pull not only floods the
repository history,
They do not flood the repository history, they accurately represent the
development history as opposed to some fabricated history that is nice
to look at but hard to find anything in.
On 02/18/2011 01:13 AM, Jeremy Mikola wrote:
It's certainly advantageous to commit early and often while developing,
but when the time comes to submit a pull request we should reduce things
to logical units. I also feel the rebasing is best done *by the author*
rather than Fabien. Merging/rebasing pull requests is tedious work, and
I'm sure his time is better spent on other tasks (like inspecting the
code :).
My point was that rebasing should only be used if no clean merge will be
possible because major changes relevant to the pull request have taken
place. So yes, the author should do so, but often it will should not be
necessary at all.
On 02/18/2011 01:13 AM, Jeremy Mikola wrote:
Additionally, the pull author is in the best position to
consolidate the commit messages during a rebase.
There is no reason to consolidate commit messages. In doing so you lose
valuable information about mistakes and process. Although I will make an
exception here, if you really just commit work in progress with broken
code etc., it does make sense to rebase those commits.
On 02/18/2011 01:13 AM, Jeremy Mikola wrote:
Lastly, I think we should use comments on pull requests to our
advantage, especially if we've submitted something that needs additional
fixes (based on peer review feedback) or if our pull depends on another,
which is also in Fabien's queue. Good communication will help avoid
cases where we have pull requests hanging in limbo and it's uncertain if
they're complete or still need corrections. I've also noticed a few
cases where pull requests were closed but it wasn't clear if the code
was accepted into the repository or not. It would be helpful if
"comment and close" was used instead of "close", even if simply
accompanied with a message of "merged" or "rejected".
I entirely agree on this part.
On Thu, Feb 17, 2011 at 6:56 PM, Nils Adermann <[email protected]
<mailto:[email protected]>> wrote:
After a discussion of various Git related problems/questions/best
practices Bernhard asked me to write up my concerns about the
current Symfony development process. I didn't feel strong enough
about any of these to bring them up before. Namely the problem I see
is an unhealthy amount of history re-writing with rebase. The
problems I believe will result from this are:
- Squashed commits are too big to identify individual changes and
the reason behind making them. Once Symfony2 in maintenance mode
this will make it more difficult to understand why a buggy piece of
code was written the way it was.
- Rebased pull requests lose information about branches that existed
during their development, resulting in commits with broken code and
failing tests. Such commits make tools like bisect difficult to use
because a problem may appear in multiple seemingly unrelated commits.
- Squashing commits of multiple authors drops authorship
information, since a commit can only ever have one commiter and one
author. Having our names listed as contributors is one of the few
rewards for contributing, so losing this information can come us a
disappointment.
In general I would like to ask for commits to be more atomic,
changing one thing rather than many to allow for easier
identification of change origins once Symfony is in its maintenance
cycle. The commits which contained bugs and problems which were
deleted through rebasing might have helped us avoid making the same
mistakes again when we later return to fix a bug. And don't get me
wrong, if you add a new feature in a commit, it can still be
significantly bigger than a bug fix commit.
How this is handled is mostly up to Fabien, so I'm wondering if you
would consider changing your workflow? I realise that the current
system seems to work well for you, but I do expect it to cause
problems in the future.
Nils
--
If you want to report a vulnerability issue on symfony, please send
it to security at symfony-project.com <http://symfony-project.com>
You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
<mailto:[email protected]>
To unsubscribe from this group, send email to
[email protected]
<mailto:symfony-devs%[email protected]>
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en
--
jeremy mikola
--
If you want to report a vulnerability issue on symfony, please send it
to security at symfony-project.com
You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en
--
If you want to report a vulnerability issue on symfony, please send it to
security at symfony-project.com
You received this message because you are subscribed to the Google
Groups "symfony developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/symfony-devs?hl=en
