Re: New Committers - community

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Hervé BOUTEMY
Le samedi 29 décembre 2018, 09:57:30 CET Mickael Istria a écrit :

> On Mon, Dec 24, 2018 at 5:21 PM Karl Heinz Marbaise <[hidden email]>
>
> wrote:
> > Hi Mickael,
>
> Hi,
>
> I have to summarize this simply.
>
> > This is no lack of interest in any kind. It is simply a lack of time of
> > the committers...cause there is very limited number of committers...
>
> I understand that, and I hope I was explicit enough when I was talking
> about the *perception* of contributions receiving low interest, as opposed
> to the reality you mention which is about lack of resources compared to
> backlog.
> In this case, some time ago, when Eclipse was in a similar position, we did
> collectively decide to make external contributions higher priority than our
> own work for a few months. This did have a very positive impact in growing
> the community, recruiting more developers, more committers, and also
> changing the mindset of most current committers to think more and more
> about incoming contributors and plan some time for the reviews, and
> prioritize community over development in some cases; because ultimately,
> the community makes the project alive more than its feature set.
>
> > > It also seems like reviewing and testing PRs is not trivial, and that
> >
> > more
> >
> > > automation could help developers to trust incoming changes and deal with
> > > reviews.
> >
> > What kind of ideas do you have? To be honest reviewing a PR takes time
> > and based on my experience no (other) automation will help there..but If
> > you have ideas please share them...
>
> I was confused about how automated tests are running or not. If I look at
> https://github.com/apache/maven/pull/194 , I do not see any build/test
> report on the issue. I imagine this requires someone to manually trigger
> the tests, doesn't it?

see https://maven.apache.org/guides/development/guide-building-maven.html

for plugins and components, running the ITs is quite easy since it's only
about activating "run-its" profile, there is no specific setup

for Maven core, it's more complex, because the its are in a separate git repo
and multiple steps to run the ITs

But in both cases, currently, there is no automatic GitHub PR integration:
Maven committers need to create a branch in the official repository to benefit
from ASF Jenkins build

Any idea how we could have GitHub PR reviews without this branch creation by
committers, be it at ASF or somewhere else?

> If it is so, then it could be one step to remove on the review process, and
> the automated build feedback would allow contributors to fix their PRs by
> themselves before wasting time of a reviewer.
>
> > > This goes by having a mindset that makes core developers main task to
> >
> > grow
> >
> > > the community rather than fixing bugs or adding features.
> >
> > That contradicts some of the feedback we got, cause we get feedback
> > saying why does it take so long to fix a bug etc...
>
> I know that. It's also something we hear often in Eclipse and I guess
> something the majority of serious OSS projects face often.
> But it's OSS, people who need a fix should be ready to invest in it; and
> someone who's finding a bug long to fix is actually someone that can be
> turned as a contributor for their own bug ;)
>
> The user community is very big but unfortunately the people who are
>
> > willing to help is not that big...
>
> Same as above ;)
>
> > That said, I think Maven already enables some important success criteria,
> >
> > > like being on GitHub. So I'm confident things can and will improve to
> >
> > grow
> >
> > > the community.
> >
> > Over the time it already evolved/grown in several aspects. Maybe not in
> > the speed we wish it had...
> > But it takes time...
>
> I have good hope it's worth it ;)





---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Enrico Olivelli
Il sab 29 dic 2018, 15:17 Stephen Connolly <[hidden email]>
ha scritto:

> There is a security issue with building PRs automatically.
>
> I can see about adding PR discovery to the existing ASF gitbox plugin, but
> we’d need committers to ok the build and have reviewed the code as the PR
> could contain attacks to be run from ASF hardware... which is why we don’t
> build PRs at present.
>

Now I have to review and then push to ASF repo and I have to tell to the
contributor that I will make CI start.
IMHO a good tradeoff is:
- a committer adds a 'test this please' comment, or '@asfbot test this
please' and then a CI job start
- this job updates the status line of the PR, with a link to the logs and
the status of the job

How does it sounds to you?

Enrico


> Other projects at ASF probably missed this point in the video series
> chronicling the development of the plugin...
>
> On Sat 29 Dec 2018 at 13:10, Enrico Olivelli <[hidden email]> wrote:
>
> > Hervè,
> > This is the plugin
> >
> >
> https://wiki.jenkins.io/display/JENKINS/GitHub+Branch+Source+Plugin?desktop=true&macroName=unmigrated-inline-wiki-markup
> >
> > I see our "maven-box" is using some special "Scan Apache Hosted Git
> > Folder Triggers" source
> > (https://builds.apache.org/job/maven-box/configure)
> > In the Jenkins of my company in a "Multibranch Pipeline" I have a
> > "Branch Sources" box with a "GitHub" option which lets me trigger
> > builds by using PRs
> >
> >
> > Enrico
> >
> > Il giorno sab 29 dic 2018 alle ore 13:43 Hervé BOUTEMY
> > <[hidden email]> ha scritto:
> > >
> > > Le samedi 29 décembre 2018, 12:40:20 CET Enrico Olivelli a écrit :
> > > > Il sab 29 dic 2018, 12:37 Mickael Istria <[hidden email]> ha
> > scritto:
> > > > > On Sat, Dec 29, 2018 at 12:01 PM Hervé BOUTEMY <
> > [hidden email]>
> > > > >
> > > > > wrote:
> > > > > > But in both cases, currently, there is no automatic GitHub PR
> > > > >
> > > > > integration:
> > > > > > Maven committers need to create a branch in the official
> > repository to
> > > > > > benefit
> > > > > > from ASF Jenkins build
> > > > >
> > > > > Ah ok, I wasn't aware the GitHub repo was "unofficial" and couldn't
> > be
> > > > > used
> > > > > to trigger builds. That sucks...
> > > >
> > > > Maven migrated to gitbox so actually github is an official repo for
> > Maven.
> > > > I see the same setup in Zookeeper and Bookkeeper and github pr plugin
> > works
> > > > like a charm (and I partecipated in setting it up)
> > > oh great, that would be nice to have the same setup for Maven repos!
> > >
> > > >
> > > > Enrico
> > > >
> > > > > Any idea how we could have GitHub PR reviews without this branch
> > creation
> > > > >
> > > > > > by
> > > > > > committers, be it at ASF or somewhere else?
> > > > >
> > > > > Using TravisCI could be a solution.
> > >
> > >
> > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> > --
> Sent from my phone
>
--


-- Enrico Olivelli
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

stephenconnolly
On Sat 29 Dec 2018 at 18:06, Vladimir Sitnikov <[hidden email]>
wrote:

> Stephen> Nah. Once committers have approved the PR then the PR can be
> merged by the
> Stephen> creator of the PR even if not a committer... at least that’s
> the default
> Stephen> way GitHub PRs work
>
> By default one needs push rights on the repository to merge PRs.
> "Approval" changes nothing. "PR approval" does not differ from any
> other comment.
> In other words, non-committers can't merge PRs.


Well on other GitHub orgs i’ve seen PR author has Merge button once PR is
approved by someone with push rights to the repo... until they add a commit
or the merge result changes

>
>
> Vladimir
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
Sent from my phone
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Vladimir Sitnikov
Stephen> Well on other GitHub orgs i’ve seen PR author has Merge
button once PR is
Stephen> approved by someone with push rights to the repo... until
they add a commit
Stephen> or the merge result changes

It does not work the way you describe.

1) By default GitHub defaults to "Base permissions" == "read".
Administrator can set even "base permissions == admin", however that
is not something that comes by default.
2) Repository administrator can configure a branch so it requires a PR
approval before merge (see
https://help.github.com/articles/enabling-required-reviews-for-pull-requests/
)

It might happen so that "author has Merge button" is a case when
repository was configured with "default=write" permissions + "require
PR approval".
Indeed that should produce (I have not checked) the behavior you
describe, however it was never a default mode for GiHub.

Vladimir

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Vladimir Sitnikov
> And, again, TravisCI also does the same in a decent way, with the benefit
> of worrying less about underlying infra and security, and only drawback of
> being discoupled from a specific infra (is it really a drawback?)

Just in case: Apache Calcite uses both Travis CI and AppVeryor. It just
works.

It does simplify validation of the changes across various Java versions,
javadoc, RAT.
AppVeyor validates Windows builds.

Both results are automatically displayed at GitHub PR (e.g.
https://github.com/apache/calcite/pull/976) with no extra manual actions.

Vladimir
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Vladimir Sitnikov
In reply to this post by Vladimir Sitnikov
Enrico> Probably Travis won't be able to run our ITs, at least the free
version

Why do you think so?

Vladimir
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Enrico Olivelli
In reply to this post by Hervé BOUTEMY
Il gio 3 gen 2019, 08:19 Hervé BOUTEMY <[hidden email]> ha scritto:

> yes, on our 100 Git repos [1], 90% should be easy to run on any CI with
> minimum configuration and resources
>
> then there is Maven core and complex plugins (like Surefire) which are not
> the
> same story
>
> if someone wants to test other toolings to show some useful feature we
> don't
> get in our current ASF Jenkins configuration [2], just start with some
> examples of the base 90% and make a demo of something that would be useful
> to
> add to our configuration
>

I would like to try
Maybe a brand new plugin like [3] scripting plugin can be a good candidate
But we need a configuration from infra and I don't know if we need a formal
vote or at least some PMC formal approval

Enrico


[3] https://github.com/apache/maven-scripting-plugin/pull/1


> Regards,
>
> Hervé
>
> [1] https://maven.apache.org/scm.html#Maven_Sources_Overview
>
> [2] https://github.com/apache/maven-jenkins-lib
>
> Le mercredi 2 janvier 2019, 15:38:12 CET Enrico Olivelli a écrit :
> > Il giorno mer 2 gen 2019 alle ore 15:34 Vladimir Sitnikov
> >
> > <[hidden email]> ha scritto:
> > > Enrico> For instance maven-surefire
> > > integration tests take 2 hours.
> > > In general I see Travis (free edition) does not offer many resources.
> > >
> > > Each Travis job is limited by 50 minutes, however one can split tests
> into
> > > multiple jobs, so it will run fine.
> >
> > I guess it won't be easy.
> > Maven integration testing is very complex, it's Maven launching Maven
> > itself. We will need to improve maven-invoker plugin, I don't know if
> there
> > is support for something like JUnit categories.
> > We can run only "unit tests" and this will be a minimal "feedback" for
> > the contributor.
> > For simpler plugins, like "checkstyle", I think Travis would be a good
> > choice, at least I would give it a try
> >
> > Enrico
> >
> > > Vladimir
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --


-- Enrico Olivelli
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

stephenconnolly
On Thu 3 Jan 2019 at 18:03, Enrico Olivelli <[hidden email]> wrote:

> Il gio 3 gen 2019, 17:38 Mickael Istria <[hidden email]> ha scritto:
>
> > Hi,
> >
> > I think this discussion is diverging into "trying TravisCI for some
> > plugins" and is loosing focus on the initial question of how to improve
> the
> > build+test flow to get faster feedback as a contributor or as a reviewer.


Travis will get the build+test flow faster for non-committers.


> > Can the GitHub PR builder plugin be enabled on Maven Core ?


No, it’s not compatible with how we build, as currently we only build from
gitbox not from GitHub so there is no link for Jenkins to see


Committers
> > would be allowed to trigger a test with a single comment once they
> checked
> > it doesn't cause a security flaw.
> >
>
> It turns out that it is not simple as it could seem because we have custom
> Jenkins plugins to scan all the repos and have a common configuration, so
> in order to achieve such goal we need to enhance that plugin, please
> Stephen correct me if I am wrong.


Yep I need to enhance the plugin a little... just trying to clear stuff off
my plate


> Travis jumped on this train because it is easy to enable and it is very
> widespread, and it leaves security problems out of ASF infra.
> But a check with Travis won't ever cover all jobs we are actually starting
> per-branch.
> It can be a good compromise to start, but if we have resources to improve
> current integration this will be the best choice for the mid term.
>
> Enrico
>
> >
> > Cheers,
> >
> --
>
>
> -- Enrico Olivelli
>
--
Sent from my phone
Reply | Threaded
Open this post in threaded view
|

Re: New Committers - community

Tibor Digana
I am doing #2.
This means I create a GitBox branch or I run it on my own PC. Not a problem
at all because usually the contributor does not complete it and I have to
add some test or clarify the change in documentation. Sometime I have to
rework the PR even if the idea was good, and I have to squash the commits.
So I am quite patient while triggering the CI via a branch but I cannot
represent the colleagues of course. Running PRs automatically would be
really a good idea.
T

On Thu, Jan 3, 2019 at 10:45 PM Stephen Connolly <
[hidden email]> wrote:

> On Thu 3 Jan 2019 at 21:37, Tibor Digana <[hidden email]> wrote:
>
> > Why the build cannot be triggered on GitHub or PR?
> > It's only a URL.
> > If you see the frequency of PRs in Surefire, 1 or 2 PRs/month, this
> should
> > not be a problem since the Windows build takes 1.5 hour and Linux 1 hour.
> > Taking JDK 7 and 11, and one Maven version (3.5) is usually enough on Git
> > branches and should be fine for PRs as well.
>
>
> 1. The current branch source only discovers branches in gitbox, it doesn’t
> discover PRs on GitHub.
>
> 2. PRs on GitHub can have “untrusted” code, which shouldn’t be run within
> the ASF hardware without review by a committer.
>
> #1 can be solved by me enhancing the Jenkins plugin we use
>
> #2 either means asking committers to trigger builds after reviewing (which
> is a manual step and manual steps get forgotten)
>
> So instead we get Travis to do a first round CI for the PRs on GitHub. Then
> we can still have #1 and #2 as final pre-merge confirmation... but Travis
> gives the contributor immediate feedback on their contribution and that
> helps grow the community.
>
>
> >
> > On Thu, Jan 3, 2019 at 10:19 PM Stephen Connolly <
> > [hidden email]> wrote:
> >
> > > On Thu 3 Jan 2019 at 20:52, Tibor Digana <[hidden email]>
> wrote:
> > >
> > > > I already lost the point of this thread.
> > > > Still the disk space problem on Windows executors in ASF Jenkins?
> > > > If it's this issue, then why the workspace is not investigated
> directly
> > > on
> > > > the system with remote access?
> > > > Jenkins is very good tool and I do not want to use Travis but Travis
> is
> > > one
> > > > step after finding the root cause.
> > >
> > >
> > > We’re not going to be able to use Travis for Surefire (unless we have a
> > > “fast” suite of tests)
> > >
> > > That doesn’t mean we cannot use Travis for the 90 other repos we have
> in
> > > order to get test results of PRs from non-committers.
> > >
> > >
> > > > T
> > > >
> > > > On Thu, Jan 3, 2019 at 8:39 PM Stephen Connolly <
> > > > [hidden email]> wrote:
> > > >
> > > > > On Thu 3 Jan 2019 at 18:03, Enrico Olivelli <[hidden email]>
> > > wrote:
> > > > >
> > > > > > Il gio 3 gen 2019, 17:38 Mickael Istria <[hidden email]> ha
> > > > scritto:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I think this discussion is diverging into "trying TravisCI for
> > some
> > > > > > > plugins" and is loosing focus on the initial question of how to
> > > > improve
> > > > > > the
> > > > > > > build+test flow to get faster feedback as a contributor or as a
> > > > > reviewer.
> > > > >
> > > > >
> > > > > Travis will get the build+test flow faster for non-committers.
> > > > >
> > > > >
> > > > > > > Can the GitHub PR builder plugin be enabled on Maven Core ?
> > > > >
> > > > >
> > > > > No, it’s not compatible with how we build, as currently we only
> build
> > > > from
> > > > > gitbox not from GitHub so there is no link for Jenkins to see
> > > > >
> > > > >
> > > > > Committers
> > > > > > > would be allowed to trigger a test with a single comment once
> > they
> > > > > > checked
> > > > > > > it doesn't cause a security flaw.
> > > > > > >
> > > > > >
> > > > > > It turns out that it is not simple as it could seem because we
> have
> > > > > custom
> > > > > > Jenkins plugins to scan all the repos and have a common
> > > configuration,
> > > > so
> > > > > > in order to achieve such goal we need to enhance that plugin,
> > please
> > > > > > Stephen correct me if I am wrong.
> > > > >
> > > > >
> > > > > Yep I need to enhance the plugin a little... just trying to clear
> > stuff
> > > > off
> > > > > my plate
> > > > >
> > > > >
> > > > > > Travis jumped on this train because it is easy to enable and it
> is
> > > very
> > > > > > widespread, and it leaves security problems out of ASF infra.
> > > > > > But a check with Travis won't ever cover all jobs we are actually
> > > > > starting
> > > > > > per-branch.
> > > > > > It can be a good compromise to start, but if we have resources to
> > > > improve
> > > > > > current integration this will be the best choice for the mid
> term.
> > > > > >
> > > > > > Enrico
> > > > > >
> > > > > > >
> > > > > > > Cheers,
> > > > > > >
> > > > > > --
> > > > > >
> > > > > >
> > > > > > -- Enrico Olivelli
> > > > > >
> > > > > --
> > > > > Sent from my phone
> > > > >
> > > >
> > > --
> > > Sent from my phone
> > >
> >
> --
> Sent from my phone
>