OK for starters I think you're wrong and I will show that. But before doing so, just for laughs, I want to ask you to set my sensitivity gauge on how I shall proceed:
[ ] whisper sweet nothings into my ear, love me and be gentle
[ ] let me know what I'm doing wrong but don't embarrass me
[ ] be brutally honest potentially revealing my incompetence
[ ] rip into me like a porn star
I am presuming you comment "Can't wait to jump to Step 4 phase !!! :)" means you want me to upload my porn star firmware :). You're a brave man Emmanuel Lecharny - but foolish.
BTW this was just for laughs on an otherwise boring topic.
OK I'm going to pretend you're that French race car driver in Talladega Nights and I'm Ricky Bobby especially since I live in Florida ... In the famous words of Ricky Bobby, "Shake and bake!"
<seriously disclaimer="sorry gotta be occasionally">
Before going into your points below I want to state the personal rules I try to operate by on commits.
(1) Always try not to break the build and tests (unit and integration) on commits to public branches and trunks - if I cannot do this I create a temporary private branch so my tinkering does not cost others pain and grief.
(2) Try to commit coherent chunks of work in a single transaction which are easy to track. This makes it so I can add a one liner summarizing the general gist of the change. For example it may be something like "Adding support for varargs in Entry API ...". Then I can have some bullet points describing the specifics of the changes if they are noteworthy.
(3) Avoid mixing bogus formatting/refactoring changes with bug fixes or feature work. When refactoring changes like class renaming or formatting I make sure this is in a separate commit so it does not obscure the diff of these other kinds of changes.
(4) Don't pick at unrelated issues. Focus on the objective. Make the changes and commit with a coherent focus message and diff so the change is easy to understand. I can relax later and pick at things like changing the code to support generics or something I saw that was not right but unrelated. Use JIRA to note things or a notepad and get back to them after you have reached your objective and committed the code.
The overall goal with these rules is to have a searchable, easy to understand version history without noise to obscure the picture. Apache projects in general promote this with unofficial rules like don't use tabs but use spaces - this is so whitespace variance to does not produce bogus chunks in diffs. This is just one example. Noise or chatter should be reduced because there's a lot of information to be filtered.
On Tue, Mar 18, 2008 at 12:41 PM, Emmanuel Lecharny <email@example.com
Alex Karasulu wrote:I don't think so. First because when you commit, you usually have
> > - - How often shall a build be done (compile/test, sitegeneration)
> We have many possible options. We tried something like kicking CI
> after each commit, but it leads to issues (usually, we don't commit
> code in one big shot,
> Yes this does happen but it's bad practice on our part.
already checked locally that the server is ok (_usually_ => sometime,
this is not the case ;).
We can't trust that. This is the reason in the first place for setting up a CI server.
Although I will entertain you by actually addressing your points, I really don't have to since you've repeatedly broken the damn build more than all of us combined. Problem is, you write some good code so we let you commit anyway. Secretly we all talk behind your back. "Oh man Emmanuel broke the damn build again. When is he going to start committing things properly. I guess we just have to live with it since he does so much for the project, makes the server fast and buys everyone beers at ApacheCon."
Another reason why I take the time is because you live with my faults too which are much worse and I am thankful for your patience. I owe you that as this is a meritocracy as well as many other things. This is the great thing about this project.
Second, because doing more than one commit
allows you to comment more precisely what kind of modification you have
Emmanuel your comments clutter the repository as they do the mailing lists with sometimes unnecessary information in a volume that is overwhelming. Besides sounding like someone with peanut butter stuck to the roof of your mouth, you're constantly pointing out little details that don't matter that much. Either that or you're off topic in la la land.
Ok ok let me be a bit more serious ... I'll stop trying to make you laugh for a minute.
We don't want log messages cluttered in the svn commit log with things like,
r1 "Adding generics to Comparator arguments"
r2 "Correct spelling"
r3 "Added Javadocs to methods"
r4 "Renamed some variables for clarity"
r5 "Added some defensive checks for null arguments"
Instead you can have this in one commit like so:
r1 "Cleaning up various jdbm-store module ...
o Adding generics to private method arguments
o Correct spelling because Alex can't spell
o Added Javadocs to methods
o Renamed some private variables for clarity
o Added some defensive checks for null arguments "
Now is this mandatory? Nope not according to rule #1 which is most important. Just don't break the build and sometimes when you have to you can commit these kinds of changes one by one. This clumping of minor details into one commit just keeps the clutter down and makes searching the repository logs much easier to do. Sometimes you'll just have no choice to do these independently and that's ok but you should have the intention of keeping the noise down.
I personnally don't really like to commit a big shot of code, unless it
is really closely connected.
Right understood. Usually I like to try to have a clear concise picture of what I will do for a specific commit to restrict it to closely related changes but this is not always possible.
Just think how much more useful our logs can be to us when we're hunting down historical changes if the changes are more coherent instead of mixed, unrelated, and strewn with bogus non-critical logs.
But as I also fix some javadoc, bugs,
warnings while browsing the code, I like to commit in smaller blocks.
Yeah I understand and you can do that. Just add the javadocs and make the fix while working in the same regions sure. But you don't have to commit that separately: most likely no one knows about the bug fix you just made unless they've noticed it. Put the bug fix in a bullet point on the overall commit like, "found nasty bug in filter parsing and fixed". Another thing is you loose focus and pick at some things as if you had the same luxury when you pick at your arse. Often these things are unrelated. Like you start fixing something then start doing generics all over the place and commit something that obscures your main intentions. You gotta stop drinking so much buddy.
The collateral changes you make are very valuable and I appreciate your taking the time and effort to make them. This is all good in the sense that it makes the code more readable so please note that I'm not talking about absolute rules here. These are guidelines to follow that lead to overall better conditions on a big project. There's a lot of code and information here so let's try to make that information easy to filter.
> I prefer a build on each commit so it's easier to catch the offendingIf you kick a build after each commit, you may have many builds kicked
> commit and isolate it to a user who can be informed immediately while
> they still have a mental stack in memory.
when a lot of commits are done.
Well if you are committing like a super sonic hedge hog then yeah. That's the whole point of my email here. Let's not commit every time we dot our i's or cross our t's.
I know this is all about your "oh lo" ratings though :-). Which is cool so I might change my mind on this whole topic and start committing like a lunatic. Oh lo is showing you out committing me. I think I'll setup a cron job to just format, unformat and commit some random file just to get back on top.
I also think that it's quite rare that a
commit break the build (it happens, say, every sic months ...),
Ha ha ha you did not just set me up for this did you? Man if you're Emmanuel the build breaks when you walk by the computer. That's why we want to keep the build server here in the US far far way from your desk.
it does, being able to point the offending commit does not really helps
to fix the breakage, because the offender is generally already sleeping :)
People will break the build at different times. Usually when tired and sleepy is the case and probably occurs more often so you may have a point there. But you're dead wrong about this not being useful. Just having the shame that comes with breaking the build is a good thing. It's what brings the arrogance and sense of can't do no wrong of all of us down.
When you slip the CI server tells you you f**ked up. A slap in the face from a machine instead of me having to feel bad about telling you that you screwed up again. So if the CI works at the end of the day it cannot catch those accountable since several errors could be mixed together from different offenders. And it's their responsibility to get their asses up to fix the problem.
Plus how do we know which change broke the build.
Remember we all live in the same code base and can give each other a hard time by how we conduct ourselves. These reminders are good when we all slip - I personally want immediate notification. I want to know when I screw up and am ok if others see it. I want the pressure to force me to change.
Good engineers architect around their own faults.
We need alarms to tell us we're messing up some how. Why? Because we all do no matter who we are. I like committers who think they will f**k up better than those that think they will not because those folks are more honest and they actually double check there work. That's all I could ask for.
>Well, run the tests before committing should be enough, isn't it ?
> I personally would like to know immediately when I goofed something
> while that something is still in my head.
I do but I screw up at times. Do you run all the tests every time you commit? We need a safety net.
Don't get me wrong : I don't say that we should fragment commits as much
as possible, nor I say that knowing which commit has broke a build is
useless, I just say that a CI should be an airbag, when the integration
is the safety belt.
I like that comment a lot!
never commit without fasten your safety belt
(-Dintegration test), and in case you crash the server, the the airbac
(CI) may save your life !
Just perfect. We're not so much off base. OK I'm going to leave me nasty comments above even after reading this because he he I took the time trying to be funny.