[LinuxBIOS] "Trivial" patches

Corey Osgood corey.osgood at gmail.com
Tue Nov 20 21:51:59 CET 2007


Uwe Hermann wrote:
> On Tue, Nov 20, 2007 at 07:40:26PM +0100, Carl-Daniel Hailfinger wrote:
>   
>> On 20.11.2007 19:20, ron minnich wrote:
>>     
>>> On Nov 20, 2007 10:20 AM, Carl-Daniel Hailfinger
>>> <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>>>
>>>   
>>>       
>>>> various big code changes/additions have been committed as trivial by
>>>> others in the past, so I am considering to follow the same policy and
>>>> declare all of my future patches as trivial and commit them instantly if
>>>> I feel like it. That would surely speed up development for me.
>>>>
>>>> Comments/Flames/Applause welcome.
>>>>     
>>>>         
>>> no, you should take us to task when we make that mistake, and I'm
>>> sorry if I have done it too much myself.
>>>
>>> Let's stick to the process, and try to flag violations of the process.
>>>   
>>>       
>> OK, can we decide on what should be (not) allowed, preferably as regexp
>> for the diff?
>>     
>
> Please don't over-engineer this. It's fine to just flame the committer
> who did a trivial commit without it being really trivial (yeah, I know,
> I'm guilty of this sometimes too).
>
> In the worst case, if the commit really _breaks_ something or is
> wrong and there's opposition, we can just revert it (which I did
> in the past, too, with one of my "trivial" fixes).
>
>
>   
>> Suggestion for NOT allowed stuff:
>> * Adding files (if they were forgotten in the previous non-trivial
>> commit, reuse the Ack from there)
>>     
>
> Why? I think this should be allowed. If the whole patch was acked and
> you simply forget to 'svn add' a file (i.e. commit only parts of the
> patch) it's perfectly valid to commit the forgotten file with that ACK.
>   

I THINK Carl-Daniel meant that was an exception, that if the files were
forgotten in the *commit*, not the original patch, then to use the ack
from the patch. If so, I fully agree.

>
>   
>> * Changing code unless it is a build fix and has "build fix" in the
>> commit log
>>     
>
> No, I don't think we want this to be _that_ daunting. Even code changes
> can be "trivial" (wrong word maybe, "obviously correct" might be better)
> sometimes. I don't think we should restrict this to comment changes only.
>
> Good examples are the "constify" patches, using PCI ID #defines instead of
> the hard-coded numbers, and similar stuff.
>   

These might be exceptions, they don't make any major change to the way
the code builds/runs. IMO, that should be the deciding factor: if the
patch makes any change to the way the code builds and runs, then it's
not trivial. The ONLY exception to that rule would be small fixes to
make abuild happy.

>> Checking for added files in the commit hook is easy. Finding out whether
>> a patch touches code or comments is difficult. My idea is to strip
>> comments from the file before and after modification, then run "diff
>> -uw" on both versions.
>>     
>
> Overkill, IMO. Just flame whoever did crap, in the worst case we revert
> the patch.
>
>
> Uwe.

I think if the commit hooks are easy enough to create, then we should
consider it. I think we've discussed this enough times, although this
may be the first time we've laid down rules. Should this go into the wiki?

-Corey




More information about the coreboot mailing list