[Arm-dev] NXP more driver patches on LS2080 SoC

Fri Jul 15 16:20:15 UTC 2016

On 7/15/16, 11:08 AM, "Jim Perrin" <jperrin at gmail.com on behalf of jperrin at centos.org> wrote:

>
>
>On 07/15/2016 10:04 AM, Lijun Pan wrote:
>> 
>> Hi Jim,
>> 
>> This solution is not acceptable to me.
>> You not only used "git apply", but also changed the commit message of the original patch,
>> which explains you were quite diligent, not sloppy.
>
>This is factually incorrect. Nothing in your patch was changed, only the
>manner in which it was applied. Your patch to the spec file left off
>some needed bits (the changelog and the release bump). Those changes
>were made following the apply, and all lumped in the same commit, which
>is the root of all this.
>
>> Please use “git am *Lijun’s.patch”, "git rebase -i HEAD~5" and "git push —-force” to correct this error.
>> After you do that, other users can update their git repo via “git remote update —prune;” to force a update.
>> It does not change their commit content but their SHA.
>
>And it forces them all to stop and fix it before continuing. That's the
>problem I have with it.

Definitely developers don’t need to stop.
After you fix the issue, they can just rebase their new patch on top of the fixed one.
Their will not be any conflicts while developers rebasing their patch
since the fix only changes the commit message.

>
>> I think the author name changes is a very serious problem.
>I agree, and it was quite unintentional.
>
>> Almost nobody looks at the changelog of the .spec.
>This is a dev vs ops perception. Most operators only see the changelog
>which is provided and referenced in every copy of the binary. Devs
>mostly see the git commits, because it's what they work with.

Developers not only see the commit message but also see the changed files in the git repo.
Users of the binary don’t see the git commit message.
This is not a very good excuse for your error.

>
>
>
>> What people cares is the author name in the commit message.
>See previous.
>
>
>
>> So I insist change it back to me (Lijun Pan) at the commit message not just add something in the .spec.
>
>
>If there's a way to do it without requiring a '--force' I'm happy to,
>but I won't break things further and impact the workflows for a others
>in the process.

Again, this is an excuse to stop fixing the “author" problem.
You don’t break others’s workflow if you just change the commit message.
Fixing this error is a showcase you will not tolerate such errors again.
However, this will probably happen again to other developers.

Lijun

>
>
>> 
>> 
>> On 7/14/16, 7:00 PM, "Jim Perrin" <jperrin at gmail.com on behalf of jperrin at centos.org> wrote:
>> 
>>> Taking this off-list.
>>>
>>> Hmm, it appears I was sloppy with my git. I did a 'git apply' of your
>>> patch instead of a 'git am'. No offense was intended, but since it's
>>> been pushed, fixing it would require a --force (If there's a way to
>>> rewrite the history without that, please let me know). I would rather
>>> not break the repo for other users by forcing a history rewrite, however
>>> I'm happy to add additional lines in the %changelog of the .spec
>>> referencing the patch, and that it came from you.
>>>
>>> Is that acceptable?
>>>
>>> On 07/14/2016 02:32 PM, Lijun Pan wrote:
>>>> Hi Jim,
>>>>
>>>> I see my patch being merged today. But the author was changed.
>>>> Also I find the previous usb related patch is under your name.
>>>> I request you change the author back to Me (Lijun Pan) as a respect of
>>>> my work.
>>>>
>>>> Thanks,
>>>>
>>>>
>>>>
>>>>        _/                                                  _/_/_/_/_/
>>>>       _/           Lijun Pan                         _/         _/
>>>>      _/           347-828-1413                   _/_/_/_/_/
>>>>     _/          _/ _/ _/     _/ _/_/    _/        _/            _/_/_/_/
>>>>     _/_/    _/
>>>>    _/          _/ _/ _/     _/ _/  _/  _/        _/            _/     _/
>>>>      _/  _/  _/
>>>>   _/_/_/_/ _/ _/ _/_/_/_/ _/    _/_/        _/            _/_/_/_/_/  
>>>> _/    _/_/
>>>>                 _/
>>>>               _/
>>>>      _/    _/
>>>>       _/_/
>>>>
>>>> On Wed, Jun 29, 2016 at 10:00 PM, Jim Perrin <jperrin at centos.org
>>>> <mailto:jperrin at centos.org>> wrote:
>>>>
>>>>
>>>>
>>>>     On 06/27/2016 05:49 PM, Lijun Pan wrote:
>>>>
>>>>     > I have reworded #8 in a more understandable way. #8 is necessary
>>>>     > for all the FSL_IFC related configuration on ARM64. #8 only affects
>>>>     > FSL_IFC,
>>>>     > not affecting other vendor's drivers. That being said, #8 is harmless.
>>>>     > I have taken #9 out.
>>>>     > I have attached version 2 of the patch.
>>>>
>>>>
>>>>     Looks good. Merged and built as the .29 kernel. This should show up as
>>>>     an update soon.
>>>>
>>>>     --
>>>>     Jim Perrin
>>>>     The CentOS Project | http://www.centos.org
>>>>     twitter: @BitIntegrity | GPG Key: FA09AD77
>>>>     _______________________________________________
>>>>     Arm-dev mailing list
>>>>     Arm-dev at centos.org <mailto:Arm-dev at centos.org>
>>>>     https://lists.centos.org/mailman/listinfo/arm-dev
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Arm-dev mailing list
>>>> Arm-dev at centos.org
>>>> https://lists.centos.org/mailman/listinfo/arm-dev
>>>>
>>>
>>> -- 
>>> Jim Perrin
>>> The CentOS Project | http://www.centos.org
>>> twitter: @BitIntegrity | GPG Key: FA09AD77
>
>-- 
>Jim Perrin
>The CentOS Project | http://www.centos.org
>twitter: @BitIntegrity | GPG Key: FA09AD77