On 10/31/2017 01:01 AM, Amar Tumballi wrote:
Possible reason it could happen, and possible suggestions, feel free to
voice your concerns:
* If people are doing 'rebase' only to check centos regressions:
o then I guess they should be told that they can probably be able
to run the tests locally too.
o is there a documentation on how to run the test locally? if not,
lets write one.
Slightly orthogonal but relevant to the issue raised above,
A rebase or a recheck should have some written justification to
understand if the contributor is attempting it after an analysis of the
failure. This helps in understanding that enough diligence is done and
possibly also raises some alerts for spurious failures that need attention.
fstat [1] helps tracking the various failures and enables us to handle
failures as a broad aspect, but even then we need the above.
So to your point, yes education is a must, but in addition to the above,
also educating on leaving behind a comment or two when rechecking would
be helpful.
[1]
https://fstat.gluster.org/summary?start_date=2017-10-01&end_date=2017-10-31&branch=master
* If 'rebase' happens mostly to resolve merge conflicts:
o very probable with patches which are big, and things which touch
many files.
o then maintainers of the components, or overall maintainers can
step up and take call.
o A request email for fastening the review process would also help.
I agree on the 3rd bullet. What are the expectations from maintainers in
the second bullet is not clear to me, can you elaborate? I understand
this as maintainers can themselves do the conflict merge and hasten the
process, if so I agree.
* If 'rebase' happens because maintainers like more than 50% of the
code (ie, idea, logic etc), but there are dis-agreements on few pieces:
o Can happen with a new contributor as they would be coming from
contributing to different project and the method we follow may
be different.
o Different maintainers have different opinions on how to use
macros, how to define variables etc etc..
o In this case, I suggest maintainers can send a message to
author, and send an updated patch with their suggestion (with
making sure '--author' is set to original author). This can save
both the effort of review, and also heart burn of someone not
understanding the comments properly.
o Documenting that this can happen also means a developer wouldn't
feel bad, and would learn from watching the actual diff between
his last change, and the change maintainer did.
o This also means, a work which may be critical for the project
doesn't take very long to land into project. Reminder, stability
and progress of the project should be our utmost priority.
o Note that I have seen at least 5 such instances where if the
reviewer picked up the patch, and worked on it, it could have
been faster.
o Again, this is not good to repeat always from a developer.
Should be OK for a contributor who is less than an year into the
component (Considering different maintainers have different
choices). Not ideal for someone older than that to the project.
But it should be maintainer's call.
I agree to the above, that, maintainers can refresh patches to address
possible common concerns, or maybe even to show alternate approach in
areas. Like Jeff stated, we possibly need some
documentation/clarifications around the same.
Let me know what you all think? I would also like to talk about it in
maintainer's meeting day after.
Regards,
Amar
_______________________________________________
maintainers mailing list
[email protected]
http://lists.gluster.org/mailman/listinfo/maintainers
_______________________________________________
maintainers mailing list
[email protected]
http://lists.gluster.org/mailman/listinfo/maintainers
