Proposal: enforcing CI checks for pull requests

[Robert Munteanu <ro...@apache.org>]

Hi,

I have drafted a proposal about enforcing CI checks for pull requests
[1]. I don't consider it fully ready as it includes a section of
classifying the Sling modules which is going to need more attention.

I'd welcome early feedback nevertheless, to check whether this looks
like the direction in which we should be heading.

Thanks for your inputs,
Robert

[1]:
https://cwiki.apache.org/confluence/display/SLING/Enforcing+CI+checks+for+pull+requests

Re: Proposal: enforcing CI checks for pull requests

[Robert Munteanu <ro...@apache.org>]

Hi Konrad,

On Mon, 2023-06-12 at 11:26 +0200, Konrad Windszus wrote:
> Hi Robert,
> 
> What issue are we trying to solve here? Is this only about preventing
> someone to press the merge button in the GitHub UI too soon?
> Did that happen in the past?

I don't think we press merge too quickly or that we are 'lax' in the
quality front. If anything, we are sometimes too slow to react to
contributions.

There are multiple issues that I think this proposal tackles:

1. Enable auto-merge

There are 'obviously correct PRs' that we can auto-merge after a quick
review. I keep coming back to renovate PRs and it's annoying :-)

2. Communicating how strict we can be with contributions to a certain
module. 

For instance, merging a change to the Sling Engine is not the same as
merging one to the Commiter CLI. I think some of us know this from
experience, but it's good to formalise it. This will help both
committers ( is it safe to merge? ) and contributors ( am I expected to
fix something? ).

> Too me this right now sounds a bit like over engineering, as the
> proposed mandatory checks are not always easy to pass (sometimes even
> impossible).

Can you give specific examples? I think we can be flexible on the rules
we apply, classifying the modules is the first step.

> So there must be a way to override those rules. 

I made a note in the open topics for the proposal.

> Is the information provided by the status checks not enough for every
> responsible Sling committer to decide whether to merge or not?

I think all Sling committers are responsible :-)

I also think we don't provide enough information about how sensitive a
certain module is, and clarifying that upfront is a good thing. I hope
that this will make it easier to accept contributions with confidence.

Expanding on that, I think it makes little sense to merge a
contribution if the Jenkins builds fail. Yes, they may be already
broken, but then again we can't validate a contribution if the build
does not pass. So it also forces us to gradually improve the technical
standing of our modules.

Hope this provides enough context.

Thanks,
Robert

> Or is this really only about auto-merge?
> I have the feeling I kind of missed the point of the proposal, maybe
> you can elaborate a bit more.
> 
> Thanks,
> Konrad
> 
> > On 5. Jun 2023, at 16:49, Robert Munteanu <ro...@apache.org>
> > wrote:
> > 
> > Hi,
> > 
> > I have drafted a proposal about enforcing CI checks for pull
> > requests
> > [1]. I don't consider it fully ready as it includes a section of
> > classifying the Sling modules which is going to need more
> > attention.
> > 
> > I'd welcome early feedback nevertheless, to check whether this
> > looks
> > like the direction in which we should be heading.
> > 
> > Thanks for your inputs,
> > Robert
> > 
> > [1]:
> > https://cwiki.apache.org/confluence/display/SLING/Enforcing+CI+checks+for+pull+requests
> 

Re: Proposal: enforcing CI checks for pull requests

[Konrad Windszus <kw...@apache.org>]

Hi Robert,

What issue are we trying to solve here? Is this only about preventing someone to press the merge button in the GitHub UI too soon?
Did that happen in the past?

Too me this right now sounds a bit like over engineering, as the proposed mandatory checks are not always easy to pass (sometimes even impossible).
So there must be a way to override those rules. 
Is the information provided by the status checks not enough for every responsible Sling committer to decide whether to merge or not?

Or is this really only about auto-merge?
I have the feeling I kind of missed the point of the proposal, maybe you can elaborate a bit more.

Thanks,
Konrad

> On 5. Jun 2023, at 16:49, Robert Munteanu <ro...@apache.org> wrote:
> 
> Hi,
> 
> I have drafted a proposal about enforcing CI checks for pull requests
> [1]. I don't consider it fully ready as it includes a section of
> classifying the Sling modules which is going to need more attention.
> 
> I'd welcome early feedback nevertheless, to check whether this looks
> like the direction in which we should be heading.
> 
> Thanks for your inputs,
> Robert
> 
> [1]:
> https://cwiki.apache.org/confluence/display/SLING/Enforcing+CI+checks+for+pull+requests