[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [servlet-dev] Removing restrictions on direct commits
|
On 04/11/2021 21:53, Stuart Douglas wrote:
On Fri, 5 Nov 2021 at 07:48, Mark Thomas <markt@xxxxxxxxxx
<mailto:markt@xxxxxxxxxx>> wrote:
As a first step, I have opened an issue to make the project leads
admins
- as we should according to the Eclipse Handbook. With admin karma, we
should then be able to make changes to the review requirements.
Mark
If you are an admin you can always merge even without review. The button
is red and it gives you an 'are you sure' popup, but you don't need to
wait for review.
I've just discovered that the project handbook and reality are not
exactly aligned. Admin karma is granted on request for 24/48 hours. I
have requested it so I can make the changes we have been discussing.
Mark
Stuart
On 03/11/2021 10:14, Stuart Douglas wrote:
>
>
> On Wed, 3 Nov 2021 at 20:28, Mark Thomas <markt@xxxxxxxxxx
<mailto:markt@xxxxxxxxxx>
> <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>> wrote:
>
> On 03/11/2021 01:48, Stuart Douglas wrote:
> >
> > On Wed, 3 Nov 2021 at 11:02, Greg Wilkins
<gregw@xxxxxxxxxxx <mailto:gregw@xxxxxxxxxxx>
> <mailto:gregw@xxxxxxxxxxx <mailto:gregw@xxxxxxxxxxx>>
> > <mailto:gregw@xxxxxxxxxxx <mailto:gregw@xxxxxxxxxxx>
<mailto:gregw@xxxxxxxxxxx <mailto:gregw@xxxxxxxxxxx>>>> wrote:
> >
> >
> > I kind of like having it, as we should very rarely be in a
> situation
> > where a PR must be committed ASAP.
> > If a PR really needs to be committed ASAP, then I
think project
> > admins have the power to override on a case by case
basis anyway.
> >
> >
> > None of us are admins though AFAIK.
>
> Correct.
>
> > Personally I would prefer that Mark
> > be made an admin and the 'Include Administrators' box be
> unticked. It
> > would mean that Mark could merge if he really needs
something for
> the
> > release, but it is a big red button with a warning.
>
> That would address my immediate concern regarding being able
to address
> release issues in a timely manner. The main thing I don't
like about
> this is that it gives me karma other committers don't have.
>
> > As a committer to many projects, I have often self
determined
> that
> > my changes were ready to be committed... only to be
surprised by
> > some silly omission, accidental inclusion or plain stupid
> mistake.
> > We are all fallible and even if not, then does
the extra few
> > hours required to wait for a review really hurt?
> >
> > cheers
> >
> > P.S. If you do turn it off, I'm still going to pester
others for
> > review of any of my PRs, as I for one have broken too many
> builds to
> > have any delusions of infallibility.
>
> Oh indeed. I am by no means infallible either. However, my
> experience of
> the last few years where EL does not have this restriction is
that
> always requiring it adds unnecessary friction to the project.
I am
> confident that all the committers can make good decisions on
when to
> just commit something and when a PR + discussion is required.
Even if
> they do get it wrong (very unusual in my experience) we can
always
> revert things if necessary.
>
> > The process should remain the same either way, and we should
> definitely
> > keep the 'Require a pull request before merging'
protection even
> if you
> > are allowed to merge your own PR's, just to prevent accidental
> pushes.
>
> I'm not convinced (working on multiple projects where PRs are not
> required) that accidental pushes are that much of a problem
but I could
> live with that.
>
>
> I looks like my initial proposal goes further than the
community is
> comfortable with but that the community would accept some
degree of
> relaxation. With that in mind I'm amending my proposal to the
following.
>
> Remove the GitHub enforced requirement that all PRs must be
reviewed.
> All changes will still be via PR. Committers will be expected
to use
> their judgement as to how long to allow for feedback before
merging
> a PR.
>
>
> I am ok with this, I trust other committers' judgement here, and
a PR
> also means an email notification so everyone will be aware of it.
>
> If there are questions after the fact the PR also provides a
place for
> this discussion to happen.
>
> I agree that accidental pushes are not really a problem in practice,
> however it's also that the PR provides a place where additional
metadata
> about the change can live.
>
> Stuart
>
> Mark
>
>
> >
> > Stuart
> >
> >
> >
> >
> >
> > On Wed, 3 Nov 2021 at 03:30, Mark Thomas
<markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
> <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>
> > <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
<mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>>> wrote:
> >
> > All,
> >
> > In approx 24 hours time I intend to request that
the branch
> > restrictions
> > that prevent committers committing directly to the
master
> branch
> > and
> > those that require every PR to be reviewed before
merge
> are removed.
> >
> > My reasoning is as follows:
> > - I have seen the benefits of these restrictions
not being
> > present in EL
> > - I'm expecting a number of non-substantive
changes will be
> > required to
> > successfully complete the release process and
PR + review
> > for all of
> > them will significantly slow us down
> > - Committers are perfectly capable of determining
which
> changes
> > need a
> > PR and review and which can be made directly -
and if
> they
> > get it
> > wrong changes can easily be reverted
> >
> > I was intending to propose this change after the
Jakarta 10
> > release but
> > on reflection, I think the sooner, the better.
> >
> > Thoughts? Comments? Objections?
> >
> > Mark
> > _______________________________________________
> > servlet-dev mailing list
> > servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
<mailto:servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>>
> <mailto:servlet-dev@xxxxxxxxxxx
<mailto:servlet-dev@xxxxxxxxxxx> <mailto:servlet-dev@xxxxxxxxxxx
<mailto:servlet-dev@xxxxxxxxxxx>>>
> > To unsubscribe from this list, visit
> > https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
> <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>>
> >
<https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
> <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>>>
> >
> >
> >
> > --
> > Greg Wilkins <gregw@xxxxxxxxxxx
<mailto:gregw@xxxxxxxxxxx> <mailto:gregw@xxxxxxxxxxx
<mailto:gregw@xxxxxxxxxxx>>
> <mailto:gregw@xxxxxxxxxxx <mailto:gregw@xxxxxxxxxxx>
<mailto:gregw@xxxxxxxxxxx <mailto:gregw@xxxxxxxxxxx>>>> CTO
> > http://webtide.com <http://webtide.com>
<http://webtide.com <http://webtide.com>> <http://webtide.com
<http://webtide.com>
> <http://webtide.com <http://webtide.com>>>
> > _______________________________________________
> > servlet-dev mailing list
> > servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
<mailto:servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>>
> <mailto:servlet-dev@xxxxxxxxxxx
<mailto:servlet-dev@xxxxxxxxxxx> <mailto:servlet-dev@xxxxxxxxxxx
<mailto:servlet-dev@xxxxxxxxxxx>>>
> > To unsubscribe from this list, visit
> > https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
> <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>>
> > <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
> <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>>>
> >
> >
> > _______________________________________________
> > servlet-dev mailing list
> > servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
<mailto:servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>>
> > To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
> <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>>
> >
>
> _______________________________________________
> servlet-dev mailing list
> servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
<mailto:servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>>
> To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
> <https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>>
>
>
> _______________________________________________
> servlet-dev mailing list
> servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
> To unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
>
_______________________________________________
servlet-dev mailing list
servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
To unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/servlet-dev
<https://www.eclipse.org/mailman/listinfo/servlet-dev>
_______________________________________________
servlet-dev mailing list
servlet-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/servlet-dev
