Re: [PATCH 0/6] pre-processing pipeline and transient package replacement

[Claudius Heine <claudius.heine.ext@siemens.com>]

On 24/04/2019 10.18, Maxim Yu. Osipov wrote:
> On 4/24/19 9:11 AM, Claudius Heine wrote:
>> Hi Maxim,
>>
>> On 22/04/2019 16.09, Maxim Yu. Osipov wrote:
>>> The patch set doesn't apply against current 'next' (it would be more 
>>> convenient to mention that this patchset depends on your previous 
>>> series [PATCH v2 0/8] Cleanup rootfs creation).
>>
>> I did in v1 of that patchset:
>>
>>  > this patchset contains some patches that where developed while
>>  > implementing the preprocessing image pipeline. They are universally
>>  > useful, but do prepare for the next steps.
>>
>> But you are correct that I did not repeat myself in this one.
>>
>> Maybe we should start looking into how to improve this process wise.
>>
>> Would it help to post a git url for each patchset?
>>
>> Maybe it might be possible to merge bigger patchset partially, so each 
>> commit that looks good, instead of rejecting always whole patchsets?
> 
>>
>> The reason why I split patchsets up is that I hope that those will be 
>> merged faster and thus lowering the work to constantly rebase 
>> everything on the current next. So for example if patch 5 of 8 makes a 
>> problem, just merge 1-4 and let the dev resent the fixed patchset 
>> containing just patch 5 to 8. If that would be done then I would have 
>> no need to try split patchsets up myself.
> 
> I agree that sometimes rejecting the whole patch set is not wise - but 
> it's up to patch series author to decide how to split patch sets.
> 
> Our policy states:
> 
> https://github.com/ilbers/isar/blob/master/CONTRIBUTING.md:
> 
>  > 3. Every patch should implement only one logical modification. The 
> patch granularity is up to the developer. In general, smaller patches 
> with clear description are easier to review and accept.
>  >
>  > 4. Please provide patches that logically belong together in a series. 
> And vice-versa, please do not submit unrelated patches as series.
>  >
>  > Every series should have a cover letter with brief information about:
>  >
>  > What this series does.
>  >
>  > How it was tested.
>  >
>  > Diffstat (git format-patch --cover-letter does this for you).
>  >
> 
> Definitely, I'll test the whole series and I'll delegate responsibility 
> of fixing problems (if found during testing) in patch set to the author.

Well there are different kind of patchsets, some patchsets implement 
features and some just cleanup/refactor code. The latter ones might 
contain a number of small improvements all over the code base. They are 
related in a sense that they 'do' the same (refactoring/cleanup), not 
necessarily with the same code pieces. But they might work towards 
cleaning up/refactoring/preparing the code for a bigger feature or 
rework that comes later.

So what you are saying that you would prefer instead of putting all 
those small refactoring changes together in one bigger patchset, that I 
should submit all those patches separate from each other? An then state 
that the feature/rework patchset depends on all of them?

I think that will cause more work on every ones end (especially with the 
slow CI build), but if that is what needs to be done when contributing 
to Isar, I will look into reordering my patchsets accordingly and maybe 
write some scripts to automate this pipeline for myself.

Claudius

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de