Re: [phpGroupWare-developers] Commit in official trunk ...

[Maât]

Caeies wrote:
> Hi all,
>
> Maât a écrit :
>   
>> Caeies wrote:
>>     
>>> 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
>>>  - ...
>>>   
>>>       
>> 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 :)
>>     
> 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 :)



>> 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
>>     
>
> 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 :)))))))))))



>> 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 ;)

 

> 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")