Patch merge process

Patch merge process

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

Hi All,

I thought of bringing this up for discussions, I am seeing that, in some pull requests some patches get dropped and I understand
that there might be a reason for that but that reason is not communicated in proposed patch mail often. I have experienced with some of the
pull requests myself and seen it with some others. I think we are not responding to patches as it should be. Since we follow
just mailing list as upstreaming process thats the only channel that developer is engaged w.r.t. patches and there should be a conclusion
on the mailing list itself for a given patch. So I would request to whoever along with Richard is validating the patches and bringing them to OE-Core from mailing lists to respond to the patches especially the ones that are not applied with reasons why its not applied and similarly to the ones that are applied but has been rejected by some reviewers. It would also be good to document the commit criteria somewhere, so developer can do rework and ease the pressure on maintainers. Can we explore some tools that can help here ? May be failed errors.yp.org output can be sent to list of developers whose patches has been part of staging ?

I think its extremely important to keep a developer informed and engaged about the upstreaming if we want to encourage more voluntary participation in the project.

It would be good to identify where we have bottlenecks and how can we improve upon the situation, ideas are welcome.

Thanks
-Khem


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: Patch merge process

[Richard Purdie]

On Tue, 2015-09-01 at 10:40 -0700, Khem Raj wrote:
> I thought of bringing this up for discussions, I am seeing that, in
> some pull requests some patches get dropped and I understand
> that there might be a reason for that but that reason is not
> communicated in proposed patch mail often. I have experienced with
> some of the
> pull requests myself and seen it with some others. I think we are not
> responding to patches as it should be. Since we follow
> just mailing list as upstreaming process thats the only channel that
> developer is engaged w.r.t. patches and there should be a conclusion
> on the mailing list itself for a given patch. So I would request to
> whoever along with Richard is validating the patches and bringing them
> to OE-Core from mailing lists to respond to the patches especially the
> ones that are not applied with reasons why its not applied and
> similarly to the ones that are applied but has been rejected by some
> reviewers. It would also be good to document the commit criteria
> somewhere, so developer can do rework and ease the pressure on
> maintainers. Can we explore some tools that can help here ? May be
> failed errors.yp.org output can be sent to list of developers whose
> patches has been part of staging ?
>
> I think its extremely important to keep a developer informed and
> engaged about the upstreaming if we want to encourage more voluntary
> participation in the project.
>
> It would be good to identify where we have bottlenecks and how can we
> improve upon the situation, ideas are welcome.

The problem basically comes down to this. We have:

* two people who put patches together into blocks and try and test them
* a single autobuilder instance capable of running the tests we need to 
  verify a set of patches
* test runs that take around 10 hours (substantially increased recently 
  by more test automation)
* ever increasing expectations that OE-Core master doesn't break
* ever increasing problems of 'random' autobuilder failures which seem 
  related to autobuilder load

Put all this together and its turning into a nightmare to keep things
flowing. The increased QA automation is catching a lot more issues which
is good, but its also making the cycles longer. The increased load is
also shaking out all kinds of races and qemu issues.

There is a SWAT team who is meant to help try and take the load off
Ross/I for some of the process. The processes there are being changed to
try and better help Ross/I. I'd ask anyone reading this and frustrated
with the patch merge process, when did you last help with SWAT? When did
you last help with any of the random autobuilder failures we're seeing
or with general YP bug fixing (not just bugs you yourself run into)?

One problem is that we have to test the patches in a block due to the
long test cycle. If something fails, we can't know who broke it, that is
up to someone's best guess so it can't be automated. Yes, you could send
out block emails but those tend to confuse people. Heck, even getting
the autobuilder to send email at all at the moment is proving
problematic. I also literally spent two days last week trying to fix
basic issues ensuring the autobuilder actually builds the revision we
think its building!

We are trying to send out info where and when things break and we know
the cause. Equally, some things have been removed as "best guess" and we
sometimes forget to either add them back, or follow up with a reply
about a likely failure they introduced :(. It comes down to simply being
overworked. Equally, the work in doing this is to be quite honest pretty
thankless and some of the lack of basic testing sometimes seen doesn't
really help :(.

That all said, I will do my best to try and send more informative emails
about patches...

Cheers,

Richard

Re: Patch merge process

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 5722 bytes --]


> On Sep 1, 2015, at 1:04 PM, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> 
> On Tue, 2015-09-01 at 10:40 -0700, Khem Raj wrote:
>> I thought of bringing this up for discussions, I am seeing that, in
>> some pull requests some patches get dropped and I understand
>> that there might be a reason for that but that reason is not
>> communicated in proposed patch mail often. I have experienced with
>> some of the
>> pull requests myself and seen it with some others. I think we are not
>> responding to patches as it should be. Since we follow
>> just mailing list as upstreaming process thats the only channel that
>> developer is engaged w.r.t. patches and there should be a conclusion
>> on the mailing list itself for a given patch. So I would request to
>> whoever along with Richard is validating the patches and bringing them
>> to OE-Core from mailing lists to respond to the patches especially the
>> ones that are not applied with reasons why its not applied and
>> similarly to the ones that are applied but has been rejected by some
>> reviewers. It would also be good to document the commit criteria
>> somewhere, so developer can do rework and ease the pressure on
>> maintainers. Can we explore some tools that can help here ? May be
>> failed errors.yp.org output can be sent to list of developers whose
>> patches has been part of staging ?
>> 
>> I think its extremely important to keep a developer informed and
>> engaged about the upstreaming if we want to encourage more voluntary
>> participation in the project.
>> 
>> It would be good to identify where we have bottlenecks and how can we
>> improve upon the situation, ideas are welcome.
> 
> The problem basically comes down to this. We have:
> 
> * two people who put patches together into blocks and try and test them
> * a single autobuilder instance capable of running the tests we need to
>  verify a set of patches

will results from other auto builders reporting to errors.yp.org can create some confidence

> * test runs that take around 10 hours (substantially increased recently
>  by more test automation)
> * ever increasing expectations that OE-Core master doesn't break

> * ever increasing problems of 'random' autobuilder failures which seem
>  related to auto builder load


I understand these issues, however response would engage the developers and keep them in loop even if it is delayed.

> 
> Put all this together and its turning into a nightmare to keep things
> flowing. The increased QA automation is catching a lot more issues which
> is good, but its also making the cycles longer. The increased load is
> also shaking out all kinds of races and qemu issues.
> 
> There is a SWAT team who is meant to help try and take the load off
> Ross/I for some of the process. The processes there are being changed to
> try and better help Ross/I. I'd ask anyone reading this and frustrated
> with the patch merge process, when did you last help with SWAT? When did
> you last help with any of the random autobuilder failures we're seeing
> or with general YP bug fixing (not just bugs you yourself run into)?

I think there could be better message reports that can be sent to mailing list on auto builder failures. Many devs
may not know about the YP’s build schedules and nomenclature.

I am particularly talking about OE-core, many devs may not be involved with Yocto project activities, even though those
activities may improve the quality of OE-core. Its just the reality.
many developers may folks chimes in when it breaks them with patches or reports and thats not a bad response, It
would be much worse if they abandoned to use the project.

> 
> One problem is that we have to test the patches in a block due to the
> long test cycle. If something fails, we can't know who broke it, that is
> up to someone's best guess so it can't be automated. Yes, you could send
> out block emails but those tend to confuse people. Heck, even getting
> the autobuilder to send email at all at the moment is proving
> problematic. I also literally spent two days last week trying to fix
> basic issues ensuring the autobuilder actually builds the revision we
> think its building!

I agree CI is not easy while you are also doing system integration iteratively.
while the build infra issues would remain, some of these concerns can be addressed if we were to use some code review tool
which can interface with auto builders and provide quick smoke tests on per patch bases, with sstate in place this could be achieved in minutes.

> 
> We are trying to send out info where and when things break and we know
> the cause. Equally, some things have been removed as "best guess" and we
> sometimes forget to either add them back, or follow up with a reply
> about a likely failure they introduced :(. It comes down to simply being
> overworked. Equally, the work in doing this is to be quite honest pretty
> thankless and some of the lack of basic testing sometimes seen doesn't
> really help :(.

I think its because the process is quite manual probably, you could send an auto email to patch when its picked to a staging branch
and when its deleted/changed, many review tools do this automatically. From personal experience I now believe that with a code review tool we can have lot more developers participate concretely in the code flight and since it provides good staging it scales quite well. So I would think using some review tool might help, at this time

> 
> That all said, I will do my best to try and send more informative emails
> about patches…

appreciated.

> 
> Cheers,
> 
> Richard
> 
> 


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: Patch merge process

[Richard Purdie]

On Tue, 2015-09-01 at 16:17 -0700, Khem Raj wrote:
> I understand these issues, however response would engage the
> developers and keep them in loop even if it is delayed.

I'm certainly in agreement we should give responses, just trying to
explain why its sometimes not as easy as it might first sound.

> > There is a SWAT team who is meant to help try and take the load off
> > Ross/I for some of the process. The processes there are being changed to
> > try and better help Ross/I. I'd ask anyone reading this and frustrated
> > with the patch merge process, when did you last help with SWAT? When did
> > you last help with any of the random autobuilder failures we're seeing
> > or with general YP bug fixing (not just bugs you yourself run into)?
> 
> I think there could be better message reports that can be sent to
> mailing list on auto builder failures. Many devs
> may not know about the YP’s build schedules and nomenclature.

Can you give more specific examples of what you mean?

> I am particularly talking about OE-core, many devs may not be involved
> with Yocto project activities, even though those
> activities may improve the quality of OE-core. Its just the reality.
> many developers may folks chimes in when it breaks them with patches
> or reports and thats not a bad response, It
> would be much worse if they abandoned to use the project.

I've not said it is a bad thing. What I'm asking is whether more people
can devote a bit of time to fixing things which aren't their immediate
problem? For example, Martin mentioned problems with errors.yp.org.
There are a ton of open issues with errors.yp.org (see bugzilla) and if
we fixed a few of those, the reporting systems would work a lot better
and more reliably. I am struggling to find anyone to work on that right
now :(. Its just one example of where some help from others would go
help improve the core of the project for the benefit of all.

> > One problem is that we have to test the patches in a block due to the
> > long test cycle. If something fails, we can't know who broke it, that is
> > up to someone's best guess so it can't be automated. Yes, you could send
> > out block emails but those tend to confuse people. Heck, even getting
> > the autobuilder to send email at all at the moment is proving
> > problematic. I also literally spent two days last week trying to fix
> > basic issues ensuring the autobuilder actually builds the revision we
> > think its building!
> 
> I agree CI is not easy while you are also doing system integration
> iteratively.
> while the build infra issues would remain, some of these concerns can
> be addressed if we were to use some code review tool
> which can interface with auto builders and provide quick smoke tests
> on per patch bases, with sstate in place this could be achieved in
> minutes.

Speaking as someone who deals with a lot of the patches, the whole
system rebuilds far too often. Our best tests for finding regressions
(DISTRO=poky-lsb, systemd-qa, oe-selftest, multilib builds, universe
fetcher) are also some of the slowest :(.

Most of the time Ross/I do pretest the revisions we build too, to try
and make the best use of the autobuilder time and catch simple failures.

> > We are trying to send out info where and when things break and we know
> > the cause. Equally, some things have been removed as "best guess" and we
> > sometimes forget to either add them back, or follow up with a reply
> > about a likely failure they introduced :(. It comes down to simply being
> > overworked. Equally, the work in doing this is to be quite honest pretty
> > thankless and some of the lack of basic testing sometimes seen doesn't
> > really help :(.
> 
> I think its because the process is quite manual probably, you could
> send an auto email to patch when its picked to a staging branch
> and when its deleted/changed, many review tools do this automatically.
> From personal experience I now believe that with a code review tool we
> can have lot more developers participate concretely in the code flight
> and since it provides good staging it scales quite well. So I would
> think using some review tool might help, at this time

We keep saying things like this, we even agreed to try and advance
patchwork. The trouble is *who* is going to do this? Since we discussed
patchwork, who has stepped up to try and resolve the comparatively
simple issues there? If this falls to me, I can try and do it but it
will be at the expense of merging patches for a while and I suspect a
lot of people are not going to be happy if I do that.

I do continue to believe something like patchwork with some integration
work could help a lot. I also continue to be absolutely dead against
gerrit (having tried it for another project).

As I mention in my other mail, some Intel people have looked at
patchwork a bit and at least started fixing the basic problems. We've
not got to the point yet where we're changing/enhancing any bigger
workflow pieces but we will hopefully get there eventually. If you're
not happy with the progress, it needs more people involved. We are
simply under resourced ultimately :(. 

Cheers,

Richard

Re: Patch merge process

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]

On Wed, Sep 02, 2015 at 08:31:33AM +0100, Richard Purdie wrote:
> I've not said it is a bad thing. What I'm asking is whether more people
> can devote a bit of time to fixing things which aren't their immediate
> problem?

Part of this problem is that sometimes it's almost too pain-full to get
some changes merged for issues which are my immediate problem that in
the end I have to implement them in our fork anyway, We cannot wait
months until it's merged and then few more months to get it backported
into older releases we're using.

So now it's usually fix it in the fork first and then if it doesn't look
controversial, try to upstream it, but the with much lower priority.

like:
http://lists.openembedded.org/pipermail/openembedded-core/2014-March/091222.html
http://patchwork.openembedded.org/patch/76601/ sent July 2014, went in on Mar 9 2015

I'm not saying it's yours or Ross's or anyone's fault. Or I'm faulty as well
with rejecting meta-oe patches until they pass my "bitbake world" tests
and sometimes it can be very hard to reproduce the issues found there.

But I understand the pain and frustration of contributors and agree with
Khem that in long term we have to do something about it or will start
loosing contributions even faster.

> For example, Martin mentioned problems with errors.yp.org.
> There are a ton of open issues with errors.yp.org (see bugzilla) and if
> we fixed a few of those, the reporting systems would work a lot better
> and more reliably. I am struggling to find anyone to work on that right
> now :(. Its just one example of where some help from others would go
> help improve the core of the project for the benefit of all.

I was in contact Andreea about some of these errors.yp.org issues when I
was extending/re-writting it for our purposes and fixing the issues I found
along the way, but we didn't get very far into integrating my fixes.

Since then it was re-written by Michael Wood so I cannot easily upstream
my changes and I'm not very interested in the re-write, because our version
just works.

I'll send patch which shortens too long failure logs, because it helps
reporting errors from bitbake world builds.

Cheers,

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch merge process

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 4474 bytes --]

On Tue, Sep 01, 2015 at 10:40:33AM -0700, Khem Raj wrote:
> Hi All,
> 
> I thought of bringing this up for discussions, I am seeing that, in some pull requests some patches get dropped and I understand
> that there might be a reason for that but that reason is not communicated in proposed patch mail often. I have experienced with some of the
> pull requests myself and seen it with some others. I think we are not responding to patches as it should be. Since we follow
> just mailing list as upstreaming process thats the only channel that developer is engaged w.r.t. patches and there should be a conclusion

Are you seeing this issues with patches sent for meta-oe and meta-qt5
repositories as well?

I know sometimes it takes me long time to reply, but I'm trying to send
failure logs or at least some information back when I'm not taking some
patches from master-next to master or worse dropping some patch from
master-next completely.

Second part of this problem is that sometimes there is no reply from
original submitter for months, so I'm actively updating 4 branches to
keep track of "state"

* origin/master
* origin/master-next (patches without terrible issues which seem to me at
  least worth trying on my jenkins world builds - that doesn't mean that
  there isn't NAK on ML about some other issues - but I still want to
  build test it asap)
* contrib/jansa/master (this is the actual branch used by jenkins builds,
  it's rebased on top of master-next and can contain some of my
  last-minute wip hotfixes I wanted to get into jenkins build even
  before sending them to ML)
* contrib/jansa/master-next-unresolved-review (dumping ground for
  rejected patches abandoned by their submitters, when I get fed up with
  seeing the same failure on jenkins builds and no reply from submitter
  I move it here and partially forget about it - until new version is
  sent and I can compare it here with older version - this branch is
  updated only when I'm really dumping some patches

> on the mailing list itself for a given patch. So I would request to whoever along with Richard is validating the patches and bringing them to OE-Core from mailing lists to respond to the patches especially the ones that are not applied with reasons why its not applied and similarly to the ones that are applied but has been rejected by some reviewers. It would also be good to document the commit criteria somewhere, so developer can do rework and ease the pressure on maintainers. Can we explore some tools that can help here ? May be failed errors.yp.org output can be sent to list of developers whose patches has been part of staging ?

I fully agree that some tooling is needed, especially now with jenkins
builds with 20+ failures, my intake process is greping console logs from
all 3 jenkins worlds to see if modified component was even built, then
greping the other report for possible new QA issues, then trying to find
thread on ML if some reviewer reported really blocking issues (or
something which can be easily fixed in follow-up patch) - sometimes I
open the patch on patchwork from master-next bundle and check it there,
because I still need to mark it as "Accepted" there - the git hook doesn't
work for last few years (never finds anything to close).

In the end this process is very error-prone and I often need to ask a
bit later for follow up fix for e.g. QA issues which weren't noticed,
because in the jenkins build I was using for check they were already
reused from sstate.

There is also small issue with errors.yp.org that many components fail
with so big log.do_compile that whole submission is rejected (currently
happens for qemux86* builds where webkit-efl is failing after some
changes in oe-core)

> I think its extremely important to keep a developer informed and engaged about the upstreaming if we want to encourage more voluntary participation in the project.
> 
> It would be good to identify where we have bottlenecks and how can we improve upon the situation, ideas are welcome.

I hate to say it, but for meta-oe and meta-qt5 I would like to use
gerrit - it's great to see all related information (all revisions of the
patch, all review comments, all results of verification builds) in
single location from where you do the actual submissions to official
branch when all criteria are satisfied.

Cheers,
-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch merge process

[Otavio Salvador]

On Tue, Sep 1, 2015 at 6:55 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
> I hate to say it, but for meta-oe and meta-qt5 I would like to use
> gerrit - it's great to see all related information (all revisions of the
> patch, all review comments, all results of verification builds) in
> single location from where you do the actual submissions to official
> branch when all criteria are satisfied.

Me too; we have been using Gerrit for patch management for a while and
it works very well.

If you want, we can host it at our server, by the way.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: Patch merge process

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]


> On Sep 1, 2015, at 3:05 PM, Otavio Salvador <otavio.salvador@ossystems.com.br> wrote:
> 
> On Tue, Sep 1, 2015 at 6:55 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
>> I hate to say it, but for meta-oe and meta-qt5 I would like to use
>> gerrit - it's great to see all related information (all revisions of the
>> patch, all review comments, all results of verification builds) in
>> single location from where you do the actual submissions to official
>> branch when all criteria are satisfied.
> 
> Me too; we have been using Gerrit for patch management for a while and
> it works very well.
> 
> If you want, we can host it at our server, by the way.

hmm can we use gerrit.openembedded.org or review.openembedded.org ?
may be redirected to a different hosting area underneath.

> 
> --
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: Patch merge process

[Otavio Salvador]

On Tue, Sep 1, 2015 at 7:46 PM, Khem Raj <raj.khem@gmail.com> wrote:
>
>> On Sep 1, 2015, at 3:05 PM, Otavio Salvador <otavio.salvador@ossystems.com.br> wrote:
>>
>> On Tue, Sep 1, 2015 at 6:55 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
>>> I hate to say it, but for meta-oe and meta-qt5 I would like to use
>>> gerrit - it's great to see all related information (all revisions of the
>>> patch, all review comments, all results of verification builds) in
>>> single location from where you do the actual submissions to official
>>> branch when all criteria are satisfied.
>>
>> Me too; we have been using Gerrit for patch management for a while and
>> it works very well.
>>
>> If you want, we can host it at our server, by the way.
>
> hmm can we use gerrit.openembedded.org or review.openembedded.org ?
> may be redirected to a different hosting area underneath.

If people want it, we can provide a VM for it. So OE can freely host it.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: Patch merge process

[Richard Purdie]

On Tue, 2015-09-01 at 19:51 -0300, Otavio Salvador wrote:
> On Tue, Sep 1, 2015 at 7:46 PM, Khem Raj <raj.khem@gmail.com> wrote:
> >> On Sep 1, 2015, at 3:05 PM, Otavio Salvador <otavio.salvador@ossystems.com.br> wrote:
> >> On Tue, Sep 1, 2015 at 6:55 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
> >>> I hate to say it, but for meta-oe and meta-qt5 I would like to use
> >>> gerrit - it's great to see all related information (all revisions of the
> >>> patch, all review comments, all results of verification builds) in
> >>> single location from where you do the actual submissions to official
> >>> branch when all criteria are satisfied.
> >>
> >> Me too; we have been using Gerrit for patch management for a while and
> >> it works very well.
> >>
> >> If you want, we can host it at our server, by the way.
> >
> > hmm can we use gerrit.openembedded.org or review.openembedded.org ?
> > may be redirected to a different hosting area underneath.
> 
> If people want it, we can provide a VM for it. So OE can freely host it.

In the interests of being completely clear I do not want to use gerrit
for OE-Core. I was asked to use it as part of another project and it
simply doesn't work with the way I think or handle patches. I cannot
begin to describe how much I disliked it.

I'd also note that it means moving the meta-oe repository to be under
gerrit control. One of the issues we've had in the past with it was that
it is a "all or nothing" solution where it either touches/controls
everything of you don't use it at all :/. What impact that has for other
repos in the system and consistency of the server I'm not sure.

It also doesn't really solve the problem we have with OE-Core which is
simply too many patches for the maintainers/submaintainers and test
cycles we have at the moment. Reading Martin's comments, I realised that
with OE-Core we do try and have a faster turnaround cycle, at least for
the less controversial patches and this probably increases some of the
pressure :/.

I should also mention that progress is being made with patchwork. We've
gone from many patches not being updated correctly to the interface
being better updated automatically now. Its still not perfect but its a
start. I continue to believe that patchwork can better integrated into
the workflows we have with a bit of time, and work is slowly moving
forward with that.

Cheers,

Richard

Re: Patch merge process

[Trevor Woerner]

If Richard doesn't want to use gerrit for OE-core, that doesn't exclude
others from using it for the layers they manage, does it?

Re: Patch merge process

[Khem Raj]

On Tue, Sep 8, 2015 at 11:18 AM, Trevor Woerner <twoerner@gmail.com> wrote:
> If Richard doesn't want to use gerrit for OE-core, that doesn't exclude
> others from using it for the layers they manage, does it?

yes, however, we should prefer this to run on oe domain, its better
for project and then if most of layers use it its fine too, it could
end up
with same situation as patchwork where oe-core and bitbake do not use
it actively. other layers do. its not ideal situation but its ok as
long as
its makes people productive, we could be open for options its matter
of volunteers then maintaining the infra for these services.

Re: Patch merge process

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Tue, Sep 08, 2015 at 11:39:17AM -0700, Khem Raj wrote:
> On Tue, Sep 8, 2015 at 11:18 AM, Trevor Woerner <twoerner@gmail.com> wrote:
> > If Richard doesn't want to use gerrit for OE-core, that doesn't exclude
> > others from using it for the layers they manage, does it?
> 
> yes, however, we should prefer this to run on oe domain, its better
> for project and then if most of layers use it its fine too, it could
> end up
> with same situation as patchwork where oe-core and bitbake do not use
> it actively. other layers do. its not ideal situation but its ok as
> long as
> its makes people productive, we could be open for options its matter
> of volunteers then maintaining the infra for these services.

I agree, there are not technical reasons for not using it for other
layers, but once we ask contributors to create account on gerrit and
submit all patches through gerrit for *some* layers, then these people
will ask why oe-core and bitbake submissions are different.

I should also say that I really like gerrit work-flow and review, but if
people continue to send patches to e-mail and someone else will need to
add Change-Ids to them and consolidate them into review-chains, than
it's not as great as it could be if all related people "buy-it" and use
it exclusively.

Regards,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch merge process

[Alexander Kanevskiy]

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]

On Wed, Sep 9, 2015 at 12:06 AM, Martin Jansa <martin.jansa@gmail.com>
wrote:

> On Tue, Sep 08, 2015 at 11:39:17AM -0700, Khem Raj wrote:
> > On Tue, Sep 8, 2015 at 11:18 AM, Trevor Woerner <twoerner@gmail.com>
> wrote:
> > > If Richard doesn't want to use gerrit for OE-core, that doesn't exclude
> > > others from using it for the layers they manage, does it?
> >
> > yes, however, we should prefer this to run on oe domain, its better
> > for project and then if most of layers use it its fine too, it could
> > end up
> > with same situation as patchwork where oe-core and bitbake do not use
> > it actively. other layers do. its not ideal situation but its ok as
> > long as
> > its makes people productive, we could be open for options its matter
> > of volunteers then maintaining the infra for these services.
>
> I agree, there are not technical reasons for not using it for other
> layers, but once we ask contributors to create account on gerrit and
> submit all patches through gerrit for *some* layers, then these people
> will ask why oe-core and bitbake submissions are different.
>
> I should also say that I really like gerrit work-flow and review, but if
> people continue to send patches to e-mail and someone else will need to
> add Change-Ids to them and consolidate them into review-chains, than
> it's not as great as it could be if all related people "buy-it" and use
> it exclusively.
>
>
Gerrit, despite its good ability to integrate into existing infrastructures
might be not the best solution for OE, and I must admit, gerrit is not the
best tool in the world for reviews, as there are limits in it like missing
ability to review properly series of patches.

However, for people who don't like Gerrit, there is also always a
possibility to use GitHub or something similar, like locally hosted GitLab
if OE don't want to be dependant on external service.


> Regards,
>
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>


-- 
br, Alexander Kanevskiy

[-- Attachment #2: Type: text/html, Size: 3635 bytes --]

Re: Patch merge process

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2787 bytes --]

On Thu, Sep 10, 2015 at 05:03:25PM +0300, Alexander Kanevskiy wrote:
> On Wed, Sep 9, 2015 at 12:06 AM, Martin Jansa <martin.jansa@gmail.com>
> wrote:
> 
> > On Tue, Sep 08, 2015 at 11:39:17AM -0700, Khem Raj wrote:
> > > On Tue, Sep 8, 2015 at 11:18 AM, Trevor Woerner <twoerner@gmail.com>
> > wrote:
> > > > If Richard doesn't want to use gerrit for OE-core, that doesn't exclude
> > > > others from using it for the layers they manage, does it?
> > >
> > > yes, however, we should prefer this to run on oe domain, its better
> > > for project and then if most of layers use it its fine too, it could
> > > end up
> > > with same situation as patchwork where oe-core and bitbake do not use
> > > it actively. other layers do. its not ideal situation but its ok as
> > > long as
> > > its makes people productive, we could be open for options its matter
> > > of volunteers then maintaining the infra for these services.
> >
> > I agree, there are not technical reasons for not using it for other
> > layers, but once we ask contributors to create account on gerrit and
> > submit all patches through gerrit for *some* layers, then these people
> > will ask why oe-core and bitbake submissions are different.
> >
> > I should also say that I really like gerrit work-flow and review, but if
> > people continue to send patches to e-mail and someone else will need to
> > add Change-Ids to them and consolidate them into review-chains, than
> > it's not as great as it could be if all related people "buy-it" and use
> > it exclusively.
> >
> 
> Gerrit, despite its good ability to integrate into existing infrastructures
> might be not the best solution for OE, and I must admit, gerrit is not the
> best tool in the world for reviews, as there are limits in it like missing

I disagree, we can agree on that :).

> ability to review properly series of patches.

It allows to review series of patches very well, newer versions also
show chains of reviews in better UI than older versions did.

Worse problem is that such series need to be uploaded as series not as
individual patches, because gerrit cannot automagically discover such
dependencies between patches and cherry-pick them to single branch and
resolve conflicts if there are, but if you push whole series of patches
it will show it as chain and all works correctly.

> However, for people who don't like Gerrit, there is also always a
> possibility to use GitHub or something similar, like locally hosted GitLab
> if OE don't want to be dependant on external service.

I want to use it to share status of the meta-oe patches with contributors,
running my own GitLab locally won't help with this.

Regards,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch merge process

[Alexander Kanevskiy]

[-- Attachment #1: Type: text/plain, Size: 2076 bytes --]

On Thu, Sep 10, 2015 at 5:18 PM, Martin Jansa <martin.jansa@gmail.com>
wrote:

> On Thu, Sep 10, 2015 at 05:03:25PM +0300, Alexander Kanevskiy wrote:
> > On Wed, Sep 9, 2015 at 12:06 AM, Martin Jansa <martin.jansa@gmail.com>
> > wrote:
> > Gerrit, despite its good ability to integrate into existing
> infrastructures
> > might be not the best solution for OE, and I must admit, gerrit is not
> the
> > best tool in the world for reviews, as there are limits in it like
> missing
>
> I disagree, we can agree on that :).
>
>
ok :)


> > ability to review properly series of patches.
>
> It allows to review series of patches very well, newer versions also
> show chains of reviews in better UI than older versions did.
>
> Worse problem is that such series need to be uploaded as series not as
> individual patches, because gerrit cannot automagically discover such
> dependencies between patches and cherry-pick them to single branch and
> resolve conflicts if there are, but if you push whole series of patches
> it will show it as chain and all works correctly.
>
>
might be. I haven't checked latest versions 2.10+, but versions below were
not up to the shape compared to e.g. pull-request workflow in GitHub.



> > However, for people who don't like Gerrit, there is also always a
> > possibility to use GitHub or something similar, like locally hosted
> GitLab
> > if OE don't want to be dependant on external service.
>
> I want to use it to share status of the meta-oe patches with contributors,
> running my own GitLab locally won't help with this.
>
>
GitHub's model is to fork repository, do any amount of modification in your
own copy and then open "pull-request" for review where you just specify
source/destination repositories and branches with set of changes.

Gitlab comment was practically to have similar to GitHub workflow, but
instead of public's GitHub service to have instance on openembedded.org
infrastructure, and thus requiring users to register accounts there.


-- 
br, Alexander Kanevskiy

[-- Attachment #2: Type: text/html, Size: 3154 bytes --]

Re: Patch merge process

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 4618 bytes --]

On Thu, Sep 10, 2015 at 06:23:07PM +0300, Alexander Kanevskiy wrote:
> On Thu, Sep 10, 2015 at 5:18 PM, Martin Jansa <martin.jansa@gmail.com>
> wrote:
> 
> > On Thu, Sep 10, 2015 at 05:03:25PM +0300, Alexander Kanevskiy wrote:
> > > On Wed, Sep 9, 2015 at 12:06 AM, Martin Jansa <martin.jansa@gmail.com>
> > > wrote:
> > > Gerrit, despite its good ability to integrate into existing
> > infrastructures
> > > might be not the best solution for OE, and I must admit, gerrit is not
> > the
> > > best tool in the world for reviews, as there are limits in it like
> > missing
> >
> > I disagree, we can agree on that :).
> >
> >
> ok :)
> 
> 
> > > ability to review properly series of patches.
> >
> > It allows to review series of patches very well, newer versions also
> > show chains of reviews in better UI than older versions did.
> >
> > Worse problem is that such series need to be uploaded as series not as
> > individual patches, because gerrit cannot automagically discover such
> > dependencies between patches and cherry-pick them to single branch and
> > resolve conflicts if there are, but if you push whole series of patches
> > it will show it as chain and all works correctly.
> >
> >
> might be. I haven't checked latest versions 2.10+, but versions below were
> not up to the shape compared to e.g. pull-request workflow in GitHub.

GitHub's pull-request workflow model is even worse than e-mail reviews
on ML.

It doesn't even support doing inline comments, because as soon as the
"source" branch is rebased or just amended to address the review
comments from v1, it will loose reference to all these comments, because
the commits will look "new" (different git SHA).

That's completely useless for good review, I want to compare different
revisions of the same patch, to see what comments I had before and how
they were addressed in new version. Github doesn't support anything like
that, people are using different work-arounds like opening new
pull-request for each revision (which means you need to always push to
new branch) or listing the commit SHAs inside the global "comments"
section, so that very patient reviewer can open the older revisions and
do manual diff (by switching between 2 browser tabs back and forth
trying to spot the differences or whatever).

Atlassian Stash has exactly the same problem, ReviewBoard the same, they
all think that to address review comments you just add another commit on
top of your branch to fix the issues you introduced in submitted commits
- but that's useless for code quality, bisect-ability etc. We don't want
10 garbage changes with incorrect commit messages and 11th change on top
which fixes the issues, we're doing review so that each change is
meaningful, build-able, test-able and properly documented in commit
message - that's all I want to ensure by using code review tool and only
Gerrit allows to do it efficiently.

Gerrit shows all revisions of the same patch (as long as Change-Id is
the same) in the same UI, allows you to see diff between any 2
revisions including comments on them etc).

Only people who never got used to do detailed reviews in Gerrit can say
that Github's pull-request are "review-tool".

You can see similar discussion e.g. lately in systemd ML after they
moved to github "for better reviews".

> > > However, for people who don't like Gerrit, there is also always a
> > > possibility to use GitHub or something similar, like locally hosted
> > GitLab
> > > if OE don't want to be dependant on external service.
> >
> > I want to use it to share status of the meta-oe patches with contributors,
> > running my own GitLab locally won't help with this.
> >
> >
> GitHub's model is to fork repository, do any amount of modification in your
> own copy and then open "pull-request" for review where you just specify
> source/destination repositories and branches with set of changes.
> 
> Gitlab comment was practically to have similar to GitHub workflow, but
> instead of public's GitHub service to have instance on openembedded.org
> infrastructure, and thus requiring users to register accounts there.

But we want to depend on well-maintained external service like we depend
on github.com to mirror our repositories and we ask our users to use
github mirrors instead of git.openembedded.org.

Similarly we can depend on http://gerrithub.io/ to host our gerrit
instance to save some maintenance for people with access to
openembedded.org infrastructure.

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: Patch merge process

[Otavio Salvador]

On Thu, Sep 10, 2015 at 12:39 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
> On Thu, Sep 10, 2015 at 06:23:07PM +0300, Alexander Kanevskiy wrote:
...
>> might be. I haven't checked latest versions 2.10+, but versions below were
>> not up to the shape compared to e.g. pull-request workflow in GitHub.
>
> GitHub's pull-request workflow model is even worse than e-mail reviews
> on ML.
>
> It doesn't even support doing inline comments, because as soon as the
> "source" branch is rebased or just amended to address the review
> comments from v1, it will loose reference to all these comments, because
> the commits will look "new" (different git SHA).
>
> That's completely useless for good review, I want to compare different
> revisions of the same patch, to see what comments I had before and how
> they were addressed in new version. Github doesn't support anything like
> that, people are using different work-arounds like opening new
> pull-request for each revision (which means you need to always push to
> new branch) or listing the commit SHAs inside the global "comments"
> section, so that very patient reviewer can open the older revisions and
> do manual diff (by switching between 2 browser tabs back and forth
> trying to spot the differences or whatever).
>
> Atlassian Stash has exactly the same problem, ReviewBoard the same, they
> all think that to address review comments you just add another commit on
> top of your branch to fix the issues you introduced in submitted commits
> - but that's useless for code quality, bisect-ability etc. We don't want
> 10 garbage changes with incorrect commit messages and 11th change on top
> which fixes the issues, we're doing review so that each change is
> meaningful, build-able, test-able and properly documented in commit
> message - that's all I want to ensure by using code review tool and only
> Gerrit allows to do it efficiently.
>
> Gerrit shows all revisions of the same patch (as long as Change-Id is
> the same) in the same UI, allows you to see diff between any 2
> revisions including comments on them etc).
>
> Only people who never got used to do detailed reviews in Gerrit can say
> that Github's pull-request are "review-tool".
>
> You can see similar discussion e.g. lately in systemd ML after they
> moved to github "for better reviews".

I fully support Gerrit for it. It does has improved how our team work
internally and we don't regret of moving to it.

I like GitHub but for serious review it does a bad job; as community it is nice.

>> GitHub's model is to fork repository, do any amount of modification in your
>> own copy and then open "pull-request" for review where you just specify
>> source/destination repositories and branches with set of changes.
>>
>> Gitlab comment was practically to have similar to GitHub workflow, but
>> instead of public's GitHub service to have instance on openembedded.org
>> infrastructure, and thus requiring users to register accounts there.
>
> But we want to depend on well-maintained external service like we depend
> on github.com to mirror our repositories and we ask our users to use
> github mirrors instead of git.openembedded.org.
>
> Similarly we can depend on http://gerrithub.io/ to host our gerrit
> instance to save some maintenance for people with access to
> openembedded.org infrastructure.

If desired we can host it in one of our machine and provide an
exclusive VM for OpenEmbedded to use.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: Patch merge process

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 5948 bytes --]


> On Sep 1, 2015, at 2:55 PM, Martin Jansa <martin.jansa@gmail.com> wrote:
> 
> On Tue, Sep 01, 2015 at 10:40:33AM -0700, Khem Raj wrote:
>> Hi All,
>> 
>> I thought of bringing this up for discussions, I am seeing that, in some pull requests some patches get dropped and I understand
>> that there might be a reason for that but that reason is not communicated in proposed patch mail often. I have experienced with some of the
>> pull requests myself and seen it with some others. I think we are not responding to patches as it should be. Since we follow
>> just mailing list as upstreaming process thats the only channel that developer is engaged w.r.t. patches and there should be a conclusion
> 
> Are you seeing this issues with patches sent for meta-oe and meta-qt5
> repositories as well?

Not specifically, moreover meta-openemebedded also uses patchwork, even though one way but there are updates that submitter can get through out patch flight even if there is no email conversation on mailing list.

> 
> I know sometimes it takes me long time to reply, but I'm trying to send
> failure logs or at least some information back when I'm not taking some
> patches from master-next to master or worse dropping some patch from
> master-next completely.

If its predictable thats more important, we can always work towards reducing the cycle time.

> 
> Second part of this problem is that sometimes there is no reply from
> original submitter for months, so I'm actively updating 4 branches to
> keep track of “state"

thats good and if ball is in submitter’s court then its over and you can ignore and may be mark it stale
descretion is upto maintainer if the patch is important and there are minor issues that you can fix and accept it so be it. if it
needs submitters attention and submitters doesn't follow up. Ignore it by all means.

> 
> * origin/master
> * origin/master-next (patches without terrible issues which seem to me at
>  least worth trying on my jenkins world builds - that doesn't mean that
>  there isn't NAK on ML about some other issues - but I still want to
>  build test it asap)
> * contrib/jansa/master (this is the actual branch used by jenkins builds,
>  it's rebased on top of master-next and can contain some of my
>  last-minute wip hotfixes I wanted to get into jenkins build even
>  before sending them to ML)
> * contrib/jansa/master-next-unresolved-review (dumping ground for
>  rejected patches abandoned by their submitters, when I get fed up with
>  seeing the same failure on jenkins builds and no reply from submitter
>  I move it here and partially forget about it - until new version is
>  sent and I can compare it here with older version - this branch is
>  updated only when I'm really dumping some patches

Thats good information. May be you can put it on Wiki as your patch flight process, so it can help submitters, when they have questions in this regard.


> 
>> on the mailing list itself for a given patch. So I would request to whoever along with Richard is validating the patches and bringing them to OE-Core from mailing lists to respond to the patches especially the ones that are not applied with reasons why its not applied and similarly to the ones that are applied but has been rejected by some reviewers. It would also be good to document the commit criteria somewhere, so developer can do rework and ease the pressure on maintainers. Can we explore some tools that can help here ? May be failed errors.yp.org output can be sent to list of developers whose patches has been part of staging ?
> 
> I fully agree that some tooling is needed, especially now with jenkins
> builds with 20+ failures, my intake process is greping console logs from
> all 3 jenkins worlds to see if modified component was even built, then
> greping the other report for possible new QA issues, then trying to find
> thread on ML if some reviewer reported really blocking issues (or
> something which can be easily fixed in follow-up patch) - sometimes I
> open the patch on patchwork from master-next bundle and check it there,
> because I still need to mark it as "Accepted" there - the git hook doesn't
> work for last few years (never finds anything to close).
> 

I think a tool like gerrit or phabricator might be worth exploring. Which can have multiple staging per
patch and lot of hooks. The key piece would be to tool, pre-commit start of jenkins build ideally, when a patch shows up on the tool
it can start a reduced build bitbake <changed-component>, however its a bit involved process to setup these tools


> In the end this process is very error-prone and I often need to ask a
> bit later for follow up fix for e.g. QA issues which weren't noticed,
> because in the jenkins build I was using for check they were already
> reused from sstate.
> 
> There is also small issue with errors.yp.org that many components fail
> with so big log.do_compile that whole submission is rejected (currently
> happens for qemux86* builds where webkit-efl is failing after some
> changes in oe-core)

may be a bug report for YP tinderbox.?

> 
>> I think its extremely important to keep a developer informed and engaged about the upstreaming if we want to encourage more voluntary participation in the project.
>> 
>> It would be good to identify where we have bottlenecks and how can we improve upon the situation, ideas are welcome.
> 
> I hate to say it, but for meta-oe and meta-qt5 I would like to use
> gerrit - it's great to see all related information (all revisions of the
> patch, all review comments, all results of verification builds) in
> single location from where you do the actual submissions to official
> branch when all criteria are satisfied.

I would support this, however, we need a gerrit instance setup and some volunteer to help set it up and maintain it. Any volunteers ?



[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]