Discussion:
Commit in official trunk ...
Caeies
2009-11-25 11:41:04 UTC
Permalink
Hi all,

I'm happy to see new logs in the svn. But I have to urge all devs to be
*very* carefull when commiting in the repository.
As maat suggest having an easy understable log is important. I will not
focus in this email on this, but I think that the way he proposed to
"tag" logs is a good idea.
The real focus in this message is this :
Each commit *in trunk* should be "atomic". I mean, that if somebody do a
checkout, trunk should be "usable" after each commit. You are free to
do whatever you want in your own branch (but I would recommand to stick
to this policy), but not in trunk. Reasons are :
- easily review patches
- easily merge
- no missing files for people
- ...

Thanks to all !

Caeies.
Maât
2009-11-25 18:41:14 UTC
Permalink
Post by Caeies
Hi all,
I'm happy to see new logs in the svn. But I have to urge all devs to be
*very* carefull when commiting in the repository.
As maat suggest having an easy understable log is important. I will not
focus in this email on this, but I think that the way he proposed to
"tag" logs is a good idea.
Each commit *in trunk* should be "atomic". I mean, that if somebody do a
checkout, trunk should be "usable" after each commit. You are free to
do whatever you want in your own branch (but I would recommand to stick
- easily review patches
- easily merge
- no missing files for people
- ...
I'll add also :

- stable
- tested
- reviewed by peers if the change lets some questions unanswered

Let's take an example with the following :

Log Message:
-----------
Bug fix : try to fix the get_member stuff, not sure if this is the right way to do it, but it works


Commits with some uncertainty and "i'm not sure" comments like this one
should not imho be committed straight in reference repositories but
rather in personal repositories and should trigger a call for review :)

Then, once the patch is tested/reviewed/improved, you can merge the
(perhaps improved) changes in one single commit with a
professionnal-looking comment like for example :

Bug fix : replaced the inexistant variable $this->account_id by the
existing $account_id

As we are starting from a low level of quality as far as sources and
changes control management is concerned we can close eyes a moment and
go on fixing things without too high expectations on process quality but
keep in mind that we'll need to improve this sooner or later (dunno if i
can use this for the french expression "tôt ou tard") if we want to use
svn logs to generate good looking Changelogs :)

regards,
Maât
Caeies
2009-11-25 19:12:28 UTC
Permalink
Hi all,
Post by Maât
Post by Caeies
Hi all,
I'm happy to see new logs in the svn. But I have to urge all devs to be
*very* carefull when commiting in the repository.
As maat suggest having an easy understable log is important. I will not
focus in this email on this, but I think that the way he proposed to
"tag" logs is a good idea.
Each commit *in trunk* should be "atomic". I mean, that if somebody do a
checkout, trunk should be "usable" after each commit. You are free to
do whatever you want in your own branch (but I would recommand to stick
- easily review patches
- easily merge
- no missing files for people
- ...
- stable
- tested
- reviewed by peers if the change lets some questions unanswered
-----------
Bug fix : try to fix the get_member stuff, not sure if this is the right way to do it, but it works
Commits with some uncertainty and "i'm not sure" comments like this one
should not imho be committed straight in reference repositories but
rather in personal repositories and should trigger a call for review :)
If trunk was bug free, I will be ok with this way of working.
Unfortunately, trunk is far away such a base, fixing it like this will
be easier. Note that I speak about _fixes_ and not "new features" ...
and IMHO this is a real difference.
Post by Maât
Then, once the patch is tested/reviewed/improved, you can merge the
(perhaps improved) changes in one single commit with a
Bug fix : replaced the inexistant variable $this->account_id by the
existing $account_id
Well If this is what you understand of the patch ... damn, review can
take long time :).

It's not because I take care of doubt in my commit that I'm not sure of
what I do ...
Post by Maât
As we are starting from a low level of quality as far as sources and
changes control management is concerned we can close eyes a moment and
go on fixing things without too high expectations on process quality but
keep in mind that we'll need to improve this sooner or later (dunno if i
can use this for the french expression "tôt ou tard") if we want to use
svn logs to generate good looking Changelogs :)
IMHO this should be done when merging new features... from personnal
branches.

Btw, before doing a commit I'm near always doing a svn annotate and take
a look at the people who commited in and the associated log ...

Regards,

Caeies
Maât
2009-11-25 22:49:14 UTC
Permalink
Post by Caeies
Hi all,
Post by Maât
Post by Caeies
Hi all,
I'm happy to see new logs in the svn. But I have to urge all devs to be
*very* carefull when commiting in the repository.
As maat suggest having an easy understable log is important. I will not
focus in this email on this, but I think that the way he proposed to
"tag" logs is a good idea.
Each commit *in trunk* should be "atomic". I mean, that if somebody do a
checkout, trunk should be "usable" after each commit. You are free to
do whatever you want in your own branch (but I would recommand to stick
- easily review patches
- easily merge
- no missing files for people
- ...
- stable
- tested
- reviewed by peers if the change lets some questions unanswered
-----------
Bug fix : try to fix the get_member stuff, not sure if this is the right way to do it, but it works
Commits with some uncertainty and "i'm not sure" comments like this one
should not imho be committed straight in reference repositories but
rather in personal repositories and should trigger a call for review :)
If trunk was bug free, I will be ok with this way of working.
Unfortunately, trunk is far away such a base, fixing it like this will
be easier. Note that I speak about _fixes_ and not "new features" ...
and IMHO this is a real difference.
I knew you'd answer something like this.

Th'as why i wrote the last part of my previous message.

Keep cool : we say globally the same thing :)
Post by Caeies
Post by Maât
Then, once the patch is tested/reviewed/improved, you can merge the
(perhaps improved) changes in one single commit with a
Bug fix : replaced the inexistant variable $this->account_id by the
existing $account_id
Well If this is what you understand of the patch ... damn, review can
take long time :).
It's not because I take care of doubt in my commit that I'm not sure of
what I do ...
People that do care about code being clean do also care about changelogs.

Having clean changelog gives a better image than logs saying "i dunno
what i'm doing but nevermind it seems to do the trick"

Wether you know in facts what you are doing is out of bounds... but just
for my own peace of mind it's good to know that you are not committing
random things :)))))))))))
Post by Caeies
Post by Maât
As we are starting from a low level of quality as far as sources and
changes control management is concerned we can close eyes a moment and
go on fixing things without too high expectations on process quality but
keep in mind that we'll need to improve this sooner or later (dunno if i
can use this for the french expression "tôt ou tard") if we want to use
svn logs to generate good looking Changelogs :)
IMHO this should be done when merging new features... from personnal
branches.
For big bugfixes on api with potentially heavy impact on several apps also

For patches changing the way of doing things even if the change does not
add new features also

And there are probably dozens of other cases where this approach is
good even if no new features are concerned ;)
Post by Caeies
Btw, before doing a commit I'm near always doing a svn annotate and take
a look at the people who commited in and the associated log ...
I need to make myself clear : I did not thought nor said i that you were
a poor hacker or whatever. (imho you are a really good at php coding)

It's just source control quality that i hope we all work to improve
(slowly) up to the highest level possible :))

(btw my comments were not supposed to upset you but rather to make you
smile and answer something like "ok we could work on this once the bing
cleaning phase is over")

Loading...