On 02/07/2011 01:43 AM, Sebastian Lauwers wrote:
On 6 February 2011 19:54, Alberto Mardegan<ma...@users.sourceforge.net> wrote:
Hi Mohammad,
The main problem with the patch is that it's big (and if I may say so,
also a bit messy). When Matan contributed some code to my project, I
asked him to rewrite the patches till they were of my liking, and he
was kind enough to do that. If he hadn't done that, I would have
either discarded the patches (if I was not interested in them) or
rewritten them myself.
So this is more a personal style discussion than an actual "let's get
things done" discussion.
No, you completely missed the point. As I wrote already, those two commits are
just an example of something which I would not let pass if I had been given a
chance to look at them. They are not awfully bad or anything.
My main point is about having more people looking through the code before it is
committed.
In the end, it doesn't really matter how many
commits there are (more specifically, having 200 commit for 200 lines
of change doesn't add any readability when it comes to repository
maintenance, and we'll soon see complaints that merges should also
include some form of rebasing in order to limit the amount of "noise"
generated by overly trigger-happy committers.
Of course.
I fully support the idea of proper, apt and useful commit messages,
but complaining that there are too few commits in a patch is just
plain nonsense.
Generally, yes. But in this case it's perfectly legitimate, because that patch
includes different features which have nothing to do with each other (such as
enabling the status menu in portrait mode and adding keyboard bindings).
git-blame allows to track what happened in a very useful manner.
But git-revert works only if the commits are meaningful chunks of code.
Whether that means being able to pinpoint commit index + 20 or commit
index + 30 doesn't *really* impact a lot; most people won't take the
time, or can't make sense of how people commit anyway. It also doesn't
hamper git-bisect in any way -- well, it only means that you have to
understand what is happening in code in order to find a bug, but
frankly, if you don't, you shouldn't be reviewing code anyway.
What?? What's the harm caused by code reviews??
FYI: I'm using git at work, and often receive +3k line commits. It
doesn't mean the coders or sloppy, it's just "one chunk of code that
does one feature".
Wow, happy reviewing! :-) In my case, such commits go to the trashbin.
I would like to see a coding style proposal, or a "how to use git
effectively", or something of the like. I think it would make working
with others a lot easier.
Sure. But we can think of that only if we first enable code reviews, otherwise
if it cannot be enforced I don't see much point in it.
Ciao,
Alberto
--
http://blog.mardy.it <-- geek in un lingua international!
_______________________________________________
maemo-community mailing list
maemo-community@maemo.org
https://lists.maemo.org/mailman/listinfo/maemo-community
