[LinuxBIOS] "Trivial" patches

[Carl-Daniel Hailfinger]

On 20.11.2007 21:51, Corey Osgood wrote:
> 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).
>>     

I'd like to place the burden of possibly problematic patches on the
committer, not on somebody who has to decide after the commit was made.
Reverting a patch is done much less lightly than committing a patch,
even if that patch is big and invasive.

With your reasoning, I should have committed a patch to enforce our
rules for trivial commits in the commit hooks and you would have to
argue that it should be reverted. I think adding such a commit hook is
trivial.

>>> 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.
>   

Thanks, Corey, that's what I meant. If you forgot a svn add in the
original acked patch, simply carry over signoff and ack from the
previous patch.

>>> * 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.
>   

So what would be the way? Check for non-whitespace differences after
preprocessing?

>>> 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.
>>     
> 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?
>   

I have modified the commit hooks before. As long as we can agree on what
needs an ack (and in case of doubt, require one), modifying the hook is
easy.


Regards,
Carl-Daniel