Proposal: forbid direct commits to master for core

[Kanstantsin Shautsou]

Situation: people doing reviews, blocking PRs for weeks,months,years while some people do direct commits to core master without any reviews. 

This ends to situations when master gets broken state that reflects on PR builds verification, i.e. https://github.com/jenkinsci/jenkins/commit/d86a88ab042cc55530d91e745af9e0886e8eeb79

Unreviewed changes adds chaos. While people reviewing and close to get rid of unconfigurable settings in https://github.com/jenkinsci/jenkins/pull/1914 one person is doing direct master changes https://github.com/jenkinsci/jenkins/commit/653fbdb65024b1b528e21f682172885f7111bba9

Proposal: stop doing such unreviewed changes and forbid direct master commits (either at all, either only for mentioned person). 

PS. AFAIR/AFAIK if you signed ICLA and do some questionable changes into master (here i see 2 violations) person should get core access removal, right?

[Oleg Nenashev]

Hi Kostya,

I understand your concern, but messages of such kind can be considered as a personal offense. Kohsuke is not the only person committing in such way, so it's definitely a wider problem, which requires a discussion. BTW currently there is no policy prohibiting such approach, so the direct commits are generally valid even if they smell bad.

I'm +1 on prohibiting direct pushes to the master branches for everybody and in all repos. And Jenkins core core is not an exception.
It makes the current release and changelogging approach a bit problematic, but it's another story.

if you signed ICLA and do some questionable changes into master (here i see 2 violations) person should get core access removal, right?

Nope. There is no such policy in Jenkins project. If you have any concerns about particular contributors, raise the topic to the governance meeting. It's the ONLY way for discussing such topics.

воскресенье, 20 декабря 2015 г., 17:03:40 UTC+3 пользователь Kanstantsin Shautsou написал:

[Baptiste Mathus]

+1 with all Oleg said... 

The subject might indeed be eligible to discussion, and I also think we might want to proceed with only PRs, but the way you do it... 

And the name you use for kk in CC is, well...

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/630f6633-d494-4726-8c21-d63890ce040a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

[Kanstantsin Shautsou]

On Dec 20, 2015, at 17:32, Baptiste Mathus <m...@batmat.net> wrote:

+1 with all Oleg said... 

The subject might indeed be eligible to discussion, and I also think we might want to proceed with only PRs, but the way you do it... 

And the name you use for kk in CC is, well…

Name was allowed, see meeting logs. 

2015-12-20 15:26 GMT+01:00 Oleg Nenashev <o.v.ne...@gmail.com>:

Hi Kostya, 

I understand your concern, but messages of such kind can be considered as a personal offense.

Any question can be transformed in any way you want.

Kohsuke is not the only person committing in such way, so it's definitely a wider problem, which requires a discussion. BTW currently there is no policy prohibiting such approach, so the direct commits are generally valid even if they smell bad.

Never saw anybody else, could you share more examples?

I'm +1 on prohibiting direct pushes to the master branches for everybody and in all repos. And Jenkins core core is not an exception.
It makes the current release and changelogging approach a bit problematic, but it's another story.

if you signed ICLA and do some questionable changes into master (here i see 2 violations) person should get core access removal, right?

Nope. There is no such policy in Jenkins project. If you have any concerns about particular contributors, raise the topic to the governance meeting. It's the ONLY way for discussing such topics.

That what core committers said to me when i asked about ICLA and perms. Would be glad to see documented way without double standards. 

воскресенье, 20 декабря 2015 г., 17:03:40 UTC+3 пользователь Kanstantsin Shautsou написал:

Situation: people doing reviews, blocking PRs for weeks,months,years while some people do direct commits to core master without any reviews. 

This ends to situations when master gets broken state that reflects on PR builds verification, i.e. https://github.com/jenkinsci/jenkins/commit/d86a88ab042cc55530d91e745af9e0886e8eeb79

Unreviewed changes adds chaos. While people reviewing and close to get rid of unconfigurable settings in https://github.com/jenkinsci/jenkins/pull/1914 one person is doing direct master changes https://github.com/jenkinsci/jenkins/commit/653fbdb65024b1b528e21f682172885f7111bba9

Proposal: stop doing such unreviewed changes and forbid direct master commits (either at all, either only for mentioned person). 

PS. AFAIR/AFAIK if you signed ICLA and do some questionable changes into master (here i see 2 violations) person should get core access removal, right?

-- 
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/630f6633-d494-4726-8c21-d63890ce040a%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

-- 

Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !

-- 

You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/d64UIypbzh0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS61D%2BXzO0Xd8jV2icNU27mJSTGZM5R0OM8rNme12ZnEFg%40mail.gmail.com.

[Andrew Bayer]

So, addressing a few aspects of this thread:

- I'd strongly oppose ICLA/push permission revocation for pushing directly to master. That's overly harsh.

- I do support this policy overall - I'm personally a big fan of a "Review then Commit" policy.

- There is a caveat/exception, of course - release-related commits. 

I think this is worth proposing for the next meeting - Kostya, could you add it to the agenda on the wiki? There's no need to name-and-shame specific cases of people pushing directly to master - this is a worthwhile policy to advocate even if no one was actually breaking it at this point.

A.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/D11D841C-4F09-4893-95C4-F5B146F25C88%40gmail.com.

[Kanstantsin Shautsou]

On Sunday, December 20, 2015 at 8:23:09 PM UTC+3, Andrew Bayer wrote:

So, addressing a few aspects of this thread:

- I'd strongly oppose ICLA/push permission revocation for pushing directly to master. That's overly harsh.

- I do support this policy overall - I'm personally a big fan of a "Review then Commit" policy.

- There is a caveat/exception, of course - release-related commits. 

I think this is worth proposing for the next meeting - Kostya, could you add it to the agenda on the wiki?

Done, added. Item can be corrected if named it wrong.  

There's no need to name-and-shame specific cases of people pushing directly to master - this is a worthwhile policy to advocate even if no one was actually breaking it at this point.

Hi, obviously proposing something without providing examples doesn't make sense. I masked name as much as i can (compare to thread that exposed my name publicly before) as i interested in technical side and resulted quality in processes.

[Christopher Orr]

Slightly hijacking this topic: would it be worthwhile having a similar
rule (even if it's may not be technically enforceable) for creating new
plugins?

i.e. everyone should have to go through the hosting request process,
even if they already have, by whatever means, permission to create
themselves a new repo in the GitHub organisation?

Regards,
Chris

On 20/12/15 18:22, Andrew Bayer wrote:
> So, addressing a few aspects of this thread:
>
> - I'd strongly oppose ICLA/push permission revocation for pushing
> directly to master. That's overly harsh.
> - I do support this policy overall - I'm personally a big fan of a
> "Review then Commit" policy.
> - There is a caveat/exception, of course - release-related commits.
>
> I think this is worth proposing for the next meeting - Kostya, could you
> add it to the agenda on the wiki? There's no need to name-and-shame
> specific cases of people pushing directly to master - this is a
> worthwhile policy to advocate even if no one was actually breaking it at
> this point.
>
> A.
>
>
> On Sun, Dec 20, 2015 at 9:41 AM, Kanstantsin Shautsou
> <kanstan...@gmail.com <mailto:kanstan...@gmail.com>> wrote:
>
>
>> On Dec 20, 2015, at 17:32, Baptiste Mathus <m...@batmat.net

>> <mailto:m...@batmat.net>> wrote:
>>
>> +1 with all Oleg said...
>> The subject might indeed be eligible to discussion, and I also
>> think we might want to proceed with only PRs, but the way you do
>> it...
>> And the name you use for kk in CC is, well…
> Name was allowed, see meeting logs.
>>
>> 2015-12-20 15:26 GMT+01:00 Oleg Nenashev <o.v.ne...@gmail.com

>> <mailto:o.v.ne...@gmail.com>>:

>>
>> Hi Kostya,
>>
>> I understand your concern, but messages of such kind can be
>> considered as a personal offense.
> Any question can be transformed in any way you want.
>>
>> Kohsuke is not the only person committing in such way, so it's
>> definitely a wider problem, which requires a discussion. BTW
>> currently there is no policy prohibiting such approach, so the
>> direct commits are generally valid even if they smell bad.
> Never saw anybody else, could you share more examples?
>>
>>
>> I'm +1 on prohibiting direct pushes to the master branches for
>> everybody and in all repos. And Jenkins core core is not an
>> exception.
>> It makes the current release and changelogging approach a bit
>> problematic, but it's another story.
>>
>> if you signed ICLA and do some questionable changes into
>> master (here i see 2 violations) person should get core
>> access removal, right?
>>
>>
>> Nope. There is no such policy in Jenkins project. If you have
>> any concerns about particular contributors, raise the topic to

>> the governance meeting. It's the *ONLY* way for discussing

>> such topics.
> That what core committers said to me when i asked about ICLA and
> perms. Would be glad to see documented way without double standards.
>>
>>
>>
>> воскресенье, 20 декабря 2015 г., 17:03:40 UTC+3 пользователь
>> Kanstantsin Shautsou написал:
>>
>> Situation: people doing reviews, blocking PRs for
>> weeks,months,years while some people do direct commits to
>> core master without any reviews.
>> This ends to situations when master gets broken state that
>> reflects on PR builds verification,
>> i.e. https://github.com/jenkinsci/jenkins/commit/d86a88ab042cc55530d91e745af9e0886e8eeb79
>> Unreviewed changes adds chaos. While people reviewing and
>> close to get rid of unconfigurable settings
>> in https://github.com/jenkinsci/jenkins/pull/1914 one
>> person is doing direct master
>> changes https://github.com/jenkinsci/jenkins/commit/653fbdb65024b1b528e21f682172885f7111bba9

>> <https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fjenkinsci%2Fjenkins%2Fcommit%2F653fbdb65024b1b528e21f682172885f7111bba9&sa=D&sntz=1&usg=AFQjCNGfejtJii5ClN4CxzyDP_BzWpWFag>

>>
>> Proposal: stop doing such unreviewed changes and forbid
>> direct master commits (either at all, either only for
>> mentioned person).
>>
>> PS. AFAIR/AFAIK if you signed ICLA and do some
>> questionable changes into master (here i see 2 violations)
>> person should get core access removal, right?
>>
>>
>> --
>> You received this message because you are subscribed to the
>> Google Groups "Jenkins Developers" group.
>> To unsubscribe from this group and stop receiving emails from
>> it, send an email
>> to jenkinsci-de...@googlegroups.com

>> <mailto:jenkinsci-de...@googlegroups.com>.

>> To view this discussion on the web
>> visit https://groups.google.com/d/msgid/jenkinsci-dev/630f6633-d494-4726-8c21-d63890ce040a%40googlegroups.com

>> <https://groups.google.com/d/msgid/jenkinsci-dev/630f6633-d494-4726-8c21-d63890ce040a%40googlegroups.com?utm_medium=email&utm_source=footer>.

>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>>
>>
>> --

>> Baptiste <Batmat> MATHUS - http://batmat.net <http://batmat.net/>

[Baptiste Mathus]

+1, absolutely. This would make the process more transparent.

[Kanstantsin Shautsou]

Hi, sorry, but it out of this topic. 

Jenkinsci project model is bazaar. I see no any reasons for enforcing somebody unrelated to reasons why plugin was created causing delays by asking why some company needs host some plugin. Somebody created plugin and share for mass usage, you may use it or not. Delays for development ends with cases when people start hosting plugins on their own servers.

What can be really critical for review - it’s not obvious things like pluginId (that unfortunately is hardcoded to artifactId), license, etc. Hope partially it will be possible automate by having maven enforcer plugin. 

       I'm +1 on prohibiting direct pushes to the master branches for
       everybody and in all repos. And Jenkins core core is not an
       exception.

The same for plugins, if *author* of plugin doesn’t want to use PRs, then you can’t enforce him do it (the reason why i left docker-plugin development). Core is critical for stability and relates to many devs/contributors, that’s why i created this topic/proposal.

-- 

You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/d64UIypbzh0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jenkinsci-de...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/56772E49.7000803%40orr.me.uk.

[Stephen Connolly]

If we are going to go down the road of forbidding direct committing to master and forcing people to go through PRs (let's assume we can find a way to let release commits go through) we'd need a better criteria for when we can actually merge PRs.

I see lots of PRs languishing for ages with disagreement over what to do and no clear outcome one way or the other...

We have 25 PRs that are older than Jun 1st 2014...

50 PRs that are at least 1 year old

75 PRs that are 6 months or older

Now KS, it's not CloudBees place to decide what the community wants to have merged... the community has not defined how to address these PRs...

If we are to move to PRs without direct commit then I want to see a defined process whereby no PR goes more than say 1 month without the community deciding if it is a Go / No Go on the general idea.

I am quite sure that CloudBees would be willing to help get PRs into a better state for merging if we knew that those PRs were the direction the community wanted to go. Right now we seem to end up saying "ok this is what we think, here's our contribution, do you want it?" and there is no movement further... after a while we then remove our CloudBees hat and don our core contributors hat and say something like "ah for jebus's sake, nobody else has expressed an opinion either way for the past 2-3 weeks, let's just merge it" but don't for one second think that we like doing this.

I would say that the community needs to show interest in PRs before we can switch to a PR model as the route for change.

My suggestion is that when a PR has been open for a week or so, the community should start a vote thread to decide if the change is the right direction (Go) or the wrong direction (No Go). If No Go then close the PR providing the reason... if Go then the PR author can be helped to get the PR to a mergeable quality and then we merge the change and move forward.

If that process gets all the open PRs down such that most PRs are open for no more than 1 month, then and only then would I say that preventing direct core commits might be worth pursuing... 

Just my €0.02

-Stephen 

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPbPdOb9uQr5_ebS82FeMJoBp32xRKU541rkGoMrw2YTN-apaw%40mail.gmail.com.

[Oleg Nenashev]

Slightly hijacking this topic: would it be worthwhile having a similar
rule (even if it's may not be technically enforceable) for creating new
plugins?

I'm +1 on it. My gut-feeling that there are misusages: forking of demo-only plugins, bad namings, etc.
It's enforceable. We could restrict the create repo permission to admins only.
The restriction of the IRC bot is also possible when we have a "community team".

The same for plugins, if *author* of plugin doesn’t want to use PRs, then you can’t enforce him do it (the reason why i left docker-plugin development). Core is critical for stability and relates to many devs/contributors, that’s why i created this topic/proposal.

At least it's a good place to start.

I see lots of PRs languishing for ages with disagreement over what to do and no clear outcome one way or the other...]

It's a complete off-topic in any case...
Jenkins does not belong to CloudBees as well the company does not belong to the project. All Jenkins contributors including CB-employed ones can designate their review time as they want/can. Obviously we use our working time to review "our" PRs, but it does not mean that nobody from CB reviews other PRs during his spare time. Honestly I don't see much unreviewed PRs, but if there are such ones you can always ask for a review in the ML. The old pending PRs have been also classified.

We have no "formal" PR review process in the community. If somebody drafts a proposal, it would be a good topic for the discussion.

понедельник, 21 декабря 2015 г., 2:58:34 UTC+3 пользователь Stephen Connolly написал:

[Luca Milanesio]

A few years ago the Gerrit workflow process was proposed, which can enforce a workflow on specific branches.

You can then have GitHub repos synchronised out-of-the-box.

See for instance the OpenStack project:

- Gerrit reviews: https://review.openstack.org

- GitHub: https://github.com/openstack

... or the Gerrit project itself :-)

- Gerrit reviews: https://gerrit-review.googlesource.com

- GitHub: https://github.com/gerritcodereview

- Jenkins CI on the reviews: https://gerrit-ci.gerritforge.com

It allows people to still push directly to some branches (e.g. experimental or devs) or submit PRs on GitHub ... but the overall workflow is managed by Gerrit.

Luca.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/f00a8e19-eee2-41a0-8a7a-d6d419c0da2f%40googlegroups.com.

[Kanstantsin Shautsou]

On Monday, December 21, 2015 at 2:58:34 AM UTC+3, Stephen Connolly wrote:

If we are going to go down the road of forbidding direct committing to master and forcing people to go through PRs (let's assume we can find a way to let release commits go through) we'd need a better criteria for when we can actually merge PRs.

I see lots of PRs languishing for ages with disagreement over what to do and no clear outcome one way or the other...

We have 25 PRs that are older than Jun 1st 2014...

50 PRs that are at least 1 year old

+ i closed all my >1 year old PRs when realised that 2.0 will be cosmetic. 

75 PRs that are 6 months or older

Now KS, it's not CloudBees place to decide what the community wants to have merged... the community has not defined how to address these PRs...

If we are to move to PRs without direct commit then I want to see a defined process whereby no PR goes more than say 1 month without the community deciding if it is a Go / No Go on the general idea.

I am quite sure that CloudBees would be willing to help get PRs into a better state for merging if we knew that those PRs were the direction the community wanted to go. Right now we seem to end up saying "ok this is what we think, here's our contribution, do you want it?" and there is no movement further... after a while we then remove our CloudBees hat and don our core contributors hat and say something like "ah for jebus's sake, nobody else has expressed an opinion either way for the past 2-3 weeks, let's just merge it" but don't for one second think that we like doing this.

Do you really think that community has enough core devs not already hired by CB? I have problem with classloading and only 3 guys (you are one of) may have answer, all in CB. 

I would say that the community needs to show interest in PRs before we can switch to a PR model as the route for change.

 PR instead direct commit at least provides chance to get review, i think you are expecting review in PRs and not in master tree. Such review already happens, i.e. Daniel provides comments in all PRs. 

My suggestion is that when a PR has been open for a week or so, the community should start a vote thread to decide if the change is the right direction (Go) or the wrong direction (No Go).

I may say that if it was open for 1 week, then community had enough time for review. Obviously CB relies on ability merging their payed changes faster and somebody may kick you for such blocker proposals :-)

If No Go then close the PR providing the reason... if Go then the PR author can be helped to get the PR to a mergeable quality and then we merge the change and move forward.

And if it doesn't fit into current 1.x, then wait for 10 years to get ability (One of my first contributions https://github.com/jenkinsci/jenkins/pull/902#issuecomment-134412342 )

After CB hiring not all changes can be reviewed i.e. fundamental https://github.com/jenkinsci/jenkins/pull/1936#issuecomment-162191335. Of course there was no objections for merge.

If that process gets all the open PRs down such that most PRs are open for no more than 1 month, then and only then would I say that preventing direct core commits might be worth pursuing... 

 In PRs you may have degree of go asap/wait for community, while in direct commits there is nothing than "don't care".

Just my €0.02

-Stephen 

Stephen, btw, could you attend meetings on IRC? 

[Stephen Connolly]

The current IRC time slot is not compatible with my family. I normally try to ensure somebody who can attend is willing to represent my PoV when I know there is an issue I feel strongly about

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/75c80dd2-6e06-4676-8a1e-bfb5bee688f7%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
Sent from my phone

[Kanstantsin Shautsou]

What slots are suitable for you? Meeting time shift was discussed few times but nothing changed.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CA%2BnPnMwnuHMGT2P4L6UvTzAt0GX-qeUDpdFpFaDr_W8DwTos0Q%40mail.gmail.com.

[Stephen Connolly]

Jenkins does not resolve around me. So I do not seek to try and make Jenkins community to jump to my timetable. all CloudBees employees know my time constraints anyway, so I do not feel the need to broadcast it in a public forum

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/D4E4A607-84D3-4F22-B975-535C3E9EA313%40gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Arnaud Héritier]

On Sun, Dec 20, 2015 at 6:22 PM, Andrew Bayer <andrew...@gmail.com> wrote:

So, addressing a few aspects of this thread:

- I'd strongly oppose ICLA/push permission revocation for pushing directly to master. That's overly harsh.

- I do support this policy overall - I'm personally a big fan of a "Review then Commit" policy.

+1 but only if people are rebasing simple changes before to merge. We can keep PRs/branch history for real long development stuffs but the major part of the time to have an history with thousands branches that are merged is humanely unreadable.

 

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPbPdOb9uQr5_ebS82FeMJoBp32xRKU541rkGoMrw2YTN-apaw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

-----

Arnaud Héritier

http://aheritier.net

Mail/GTalk: aheritier AT gmail DOT com

Twitter/Skype : aheritier

[Andrew Bayer]

Oh, definitely!

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAFNCU--GcAN1zS9uym-2xDMJGHFu_Tk8iJj7T3gNwLSY-FJM3Q%40mail.gmail.com.

[Surya Gaddipati]

>Jenkinsci project model is bazaar. I see no any reasons for enforcing somebody unrelated to reasons why plugin was created causing delays by asking why some company needs host some plugin. Somebody created plugin and share for mass usage, you may use it or not. Delays for development ends with cases when people start hosting plugins on their own servers.

I have a comment/proposal about this but don't want to hijack this thread as it seems offtopic. Perhaps a new topic.

[Christopher Orr]

On 21/12/15 09:33, Oleg Nenashev wrote:
> Slightly hijacking this topic: would it be worthwhile having a similar
> rule (even if it's may not be technically enforceable) for creating new
> plugins?
>
>
> I'm +1 on it. My gut-feeling that there are misusages: forking of
> demo-only plugins, bad namings, etc.
> It's enforceable. We could restrict the create repo permission to admins
> only.
> The restriction of the IRC bot is also possible when we have a
> "community team".

I'm sure some admins are also guilty of forking their own plugins.. :)

> <javascript:>> wrote:
>
> So, addressing a few aspects of this thread:
>
> - I'd strongly oppose ICLA/push permission revocation for
> pushing directly to master. That's overly harsh.
> - I do support this policy overall - I'm personally a big fan of
> a "Review then Commit" policy.
> - There is a caveat/exception, of course - release-related commits.
>
> I think this is worth proposing for the next meeting - Kostya,
> could you add it to the agenda on the wiki? There's no need to
> name-and-shame specific cases of people pushing directly to
> master - this is a worthwhile policy to advocate even if no one
> was actually breaking it at this point.
>
> A.
>
>
> On Sun, Dec 20, 2015 at 9:41 AM, Kanstantsin Shautsou
> <kanstan...@gmail.com <javascript:>> wrote:
>
>
>> On Dec 20, 2015, at 17:32, Baptiste Mathus

>> <m...@batmat.net <javascript:>> wrote:
>>
>> +1 with all Oleg said...
>> The subject might indeed be eligible to discussion, and I
>> also think we might want to proceed with only PRs, but the
>> way you do it...
>> And the name you use for kk in CC is, well…
> Name was allowed, see meeting logs.
>>
>> 2015-12-20 15:26 GMT+01:00 Oleg

>> Nenashev <o.v.ne...@gmail.com <javascript:>>:

>>
>> Hi Kostya,
>>
>> I understand your concern, but messages of such kind
>> can be considered as a personal offense.
> Any question can be transformed in any way you want.
>>
>> Kohsuke is not the only person committing in such way,
>> so it's definitely a wider problem, which requires a
>> discussion. BTW currently there is no policy
>> prohibiting such approach, so the direct commits are
>> generally valid even if they smell bad.
> Never saw anybody else, could you share more examples?
>>
>>
>> I'm +1 on prohibiting direct pushes to the master
>> branches for everybody and in all repos. And Jenkins
>> core core is not an exception.
>> It makes the current release and changelogging
>> approach a bit problematic, but it's another story.
>>
>> if you signed ICLA and do some questionable
>> changes into master (here i see 2 violations)
>> person should get core access removal, right?
>>
>>
>> Nope. There is no such policy in Jenkins project. If
>> you have any concerns about particular contributors,
>> raise the topic to the governance meeting. It's

>> the *ONLY* way for discussing such topics.

> That what core committers said to me when i asked about ICLA
> and perms. Would be glad to see documented way without
> double standards.
>>
>>
>>
>> воскресенье, 20 декабря 2015 г., 17:03:40 UTC+3
>> пользователь Kanstantsin Shautsou написал:
>>
>> Situation: people doing reviews, blocking PRs for
>> weeks,months,years while some people do direct
>> commits to core master without any reviews.
>> This ends to situations when master gets broken
>> state that reflects on PR builds verification,
>> i.e. https://github.com/jenkinsci/jenkins/commit/d86a88ab042cc55530d91e745af9e0886e8eeb79
>> <https://github.com/jenkinsci/jenkins/commit/d86a88ab042cc55530d91e745af9e0886e8eeb79>
>> Unreviewed changes adds chaos. While people
>> reviewing and close to get rid of unconfigurable
>> settings
>> in https://github.com/jenkinsci/jenkins/pull/1914
>> <https://github.com/jenkinsci/jenkins/pull/1914> one
>> person is doing direct master
>> changes https://github.com/jenkinsci/jenkins/commit/653fbdb65024b1b528e21f682172885f7111bba9

>> <https://www.google.com/url?q=https%3A%2F%2Fgithub.com%2Fjenkinsci%2Fjenkins%2Fcommit%2F653fbdb65024b1b528e21f682172885f7111bba9&sa=D&sntz=1&usg=AFQjCNGfejtJii5ClN4CxzyDP_BzWpWFag>

>>
>> Proposal: stop doing such unreviewed changes and
>> forbid direct master commits (either at all,
>> either only for mentioned person).
>>
>> PS. AFAIR/AFAIK if you signed ICLA and do some
>> questionable changes into master (here i see 2
>> violations) person should get core access removal,
>> right?
>>
>>
>> --
>> You received this message because you are subscribed
>> to the Google Groups "Jenkins Developers" group.
>> To unsubscribe from this group and stop receiving
>> emails from it, send an email

>> to jenkinsci-de...@googlegroups.com <javascript:>.

>> To view this discussion on the web
>> visit https://groups.google.com/d/msgid/jenkinsci-dev/630f6633-d494-4726-8c21-d63890ce040a%40googlegroups.com

>> <https://groups.google.com/d/msgid/jenkinsci-dev/630f6633-d494-4726-8c21-d63890ce040a%40googlegroups.com?utm_medium=email&utm_source=footer>.

>>
>> For more options,
>> visit https://groups.google.com/d/optout

>> <https://groups.google.com/d/optout>.

>>
>>
>>
>>
>> --
>> Baptiste <Batmat> MATHUS - http://batmat.net

>> <http://batmat.net/>

>> Sauvez un arbre,
>> Mangez un castor !
>>
>> --
>> You received this message because you are subscribed to a
>> topic in the Google Groups "Jenkins Developers" group.
>> To unsubscribe from this topic,
>> visit https://groups.google.com/d/topic/jenkinsci-dev/d64UIypbzh0/unsubscribe

>> <https://groups.google.com/d/topic/jenkinsci-dev/d64UIypbzh0/unsubscribe>.

>> To unsubscribe from this group and all its topics, send an

>> email to jenkinsci-de...@googlegroups.com <javascript:>.

>> To view this discussion on the web

>> visit https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS61D%2BXzO0Xd8jV2icNU27mJSTGZM5R0OM8rNme12ZnEFg%40mail.gmail.com
>> <https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS61D%2BXzO0Xd8jV2icNU27mJSTGZM5R0OM8rNme12ZnEFg%40mail.gmail.com?utm_medium=email&utm_source=footer>.

>> For more options, visit https://groups.google.com/d/optout

>> <https://groups.google.com/d/optout>.

>
> --
> You received this message because you are subscribed to the
> Google Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails
> from it, send an email to jenkinsci-de...@googlegroups.com

> <javascript:>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/jenkinsci-dev/D11D841C-4F09-4893-95C4-F5B146F25C88%40gmail.com
> <https://groups.google.com/d/msgid/jenkinsci-dev/D11D841C-4F09-4893-95C4-F5B146F25C88%40gmail.com?utm_medium=email&utm_source=footer>.

>
> For more options, visit https://groups.google.com/d/optout

> <https://groups.google.com/d/optout>.

>
>
> --
> You received this message because you are subscribed to the
> Google Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from

> it, send an email to jenkinsci-de...@googlegroups.com <javascript:>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/jenkinsci-dev/CAPbPdOb9uQr5_ebS82FeMJoBp32xRKU541rkGoMrw2YTN-apaw%40mail.gmail.com
> <https://groups.google.com/d/msgid/jenkinsci-dev/CAPbPdOb9uQr5_ebS82FeMJoBp32xRKU541rkGoMrw2YTN-apaw%40mail.gmail.com?utm_medium=email&utm_source=footer>.

>
> For more options, visit https://groups.google.com/d/optout

> <https://groups.google.com/d/optout>.

>
>
> --
> You received this message because you are subscribed to the Google
> Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to jenkinsci-de...@googlegroups.com

> <mailto:jenkinsci-de...@googlegroups.com>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/jenkinsci-dev/f00a8e19-eee2-41a0-8a7a-d6d419c0da2f%40googlegroups.com
> <https://groups.google.com/d/msgid/jenkinsci-dev/f00a8e19-eee2-41a0-8a7a-d6d419c0da2f%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Kanstantsin Shautsou]

Is it weird only for me https://github.com/jenkinsci/jenkins/pull/1301 ? Could somebody describe?

[Kanstantsin Shautsou]

Keeping just for logging of this topic to save future time during searching examples 

https://github.com/jenkinsci/jenkins/commit/bb7c8fcedbcc9b51c5b1bb5b32810af5ac6b1ffb 

[Stephen Connolly]

I think I am now against this proposal. At least without an agreed mechanism to make large cross-cutting changes without ending up in the:

1. Submit PR

2. Request review

3. Wait

4. If PR is no-longer mergable, or needs fix-up due to other PRs merged, Then fix up PR and Goto 2

5. Merge PR

steps 2-4 loop of death.

Especially with a change that is open to debate, every time it is resubmitted for review, you will run the gauntlet of trying to get everyone to agree *again*.

There are some changes which are not well suited to either a PR or a feature branch.

Find a process that can enable those changes and I am all for that process... But blanket banning direct commits without enabling movement on fixes to core issues such as the idiotic forcing all plugins to change from getInstance to getActiveInstance because <5% of the callers in core have a need to work outside of the lifecycle of the singleton and some plugins want to break things by dragging the Jenkins class over to the remote class loaders... That will just drag the code base into the tarpit of dispair

On Tuesday 8 March 2016, Kanstantsin Shautsou <kanstan...@gmail.com> wrote:

Keeping just for logging of this topic to save future time during searching examples 

https://github.com/jenkinsci/jenkins/commit/bb7c8fcedbcc9b51c5b1bb5b32810af5ac6b1ffb 

--

You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/851cc3bd-5d6b-46db-acc9-d1f04874ae8e%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Daniel Beck]

On 08.03.2016, at 21:29, Stephen Connolly <stephen.al...@gmail.com> wrote:

> 2. Request review
> 3. Wait
> 4. If PR is no-longer mergable, or needs fix-up due to other PRs merged, Then fix up PR and Goto 2

FWIW I would not consider most fix ups cause to go back to #2 if there have been positive reviews of the original version.

[Stephen Connolly]

So you are saying that somebody making fix-ups won't make mistakes and the changes do not need re-review?

It's hard enough in the current process to know at what point you have the OK to merge... 

 

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/1F7F3EB2-706E-4E38-837A-903D126442D0%40beckweb.net.

[Kanstantsin Shautsou]

On Wednesday, March 9, 2016 at 12:45:40 AM UTC+3, Stephen Connolly wrote:

On 8 March 2016 at 20:39, Daniel Beck <m...@beckweb.net> wrote:

On 08.03.2016, at 21:29, Stephen Connolly <stephen.al...@gmail.com> wrote:

> 2. Request review
> 3. Wait
> 4. If PR is no-longer mergable, or needs fix-up due to other PRs merged, Then fix up PR and Goto 2

FWIW I would not consider most fix ups cause to go back to #2 if there have been positive reviews of the original version.

So you are saying that somebody making fix-ups won't make mistakes and the changes do not need re-review?

It's hard enough in the current process to know at what point you have the OK to merge... 

I think it depends on contributors quality, somebody may request final review, somebody may merge. But that definitely better than invisible commits to master.

@Stephenc, when you are watching `jenskinci/jenkins` repository, are you getting notifications for all commits? I think no. 

How do you expect people reviewing/knowing about code changes without PRs? 

(Even if your change was right.) Do you think that your change is super important and everybody should be ignored? Or that’s personal kohsuke’s project that allows doing anything he want without caring on cooperation with other contributors that made a lot of https://github.com/jenkinsci/jenkins/graphs/contributors and testing?

This wasn't an automation or release process commit. It wasn't so difficult to make PR and keep discussions in PR thread that will have comments in it and not in commit. PR would allow ensure that build at all buildable.

I think the first small step should be to use PRs as gate of changes and degree of review could be adopted later.

[Daniel Beck]

On 08.03.2016, at 22:45, Stephen Connolly <stephen.al...@gmail.com> wrote:

> So you are saying that somebody making fix-ups won't make mistakes and the changes do not need re-review?
>
> It's hard enough in the current process to know at what point you have the OK to merge...

Of course *some* merges are more complicated, but *most* seem to be straightforward. Worst case you just grab someone on IRC and have them confirm it was merged correctly.

[Suckow, Thomas J]

Not being an iCLA signer I have experienced the loop of death and that’s fine. I do not however wish it on iCLA signers.

Would an acceptable compromise be that people with push access are allowed to optionally "skip" the human review portion of the PR? It would still require that Jenkins is able to build the PR before they are allowed to merge.

1. Submit PR

2. Wait for Jenkins to verify

3. If PR is no-longer mergable, or needs fix-up due to other PRs merged, Then fix up PR and Goto 2

4. Merge PR

Since it was a PR, people could still review it after the fact. I would also encourage them leaving it for review. But I imagine the biggest annoyance would be for bug fixes which should be lower risk and there is a high desire to get merged quickly.

-

Thomas

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CA%2BnPnMwsYad6nVBK8pp5de6Z%2BezyXpPbRMrDXZzZZY3Zm7Jttw%40mail.gmail.com.

[Richard Bywater]

Personally the way I see it possibly the issue isn't really with having a PR or not - it seems to me that the issue was more with not having a discussion first somewhere prior to committing about the approach / issue.  (Or maybe there was - in which case ignore my view :) )

Richard

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/D305962B.282AE%25thomas.suckow%40pnnl.gov.