You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Robert Houghton <rh...@pivotal.io> on 2020/05/13 21:32:16 UTC

[DISCUSS] enable GitHub PR blocking on API breaking changes

We have enabled this job on the develop and support 1.13 branches, and it
is going well. I would like this to be a blocking job for our pull
requests.

Are there any issues around this test that we want to address, or reasons
to *not* have it be a blocking job in the PR pipeline?

To seed the conversation, there is an issue I am working on with @Mario
Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.

I would like to have a [VOTE] by end of week on this.

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Robert Houghton <rh...@pivotal.io>.
INFRA has added the status to the required list for merging PRs to
geode/develop. Thanks everyone.

On Fri, May 15, 2020 at 3:03 PM Robert Houghton <rh...@pivotal.io>
wrote:

> Please refer to https://issues.apache.org/jira/browse/INFRA-20270
>
> On Fri, May 15, 2020 at 2:58 PM Robert Houghton <rh...@pivotal.io>
> wrote:
>
>> Support seems to be unanimously good, and folks voted in this thread. I'm
>> going to make the ASF-INFRA request.
>>
>> On Fri, May 15, 2020 at 11:47 AM Anthony Baker <ba...@vmware.com> wrote:
>>
>>> Barry and I tossed up a draft PR to fix a problem in session state
>>> replication with Tomcat.  If we can get this completed I’d like to include
>>> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
>>> after 9.0.21.
>>>
>>> Anthony
>>>
>>>
>>> > On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com> wrote:
>>> >
>>> > +1
>>> >
>>> > On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:
>>> >
>>> >> +1. The more we can automate this types of checks the better.
>>> >>
>>> >> Thanks,
>>> >> Mark
>>> >>
>>> >>
>>> >
>>> > --
>>> > Ju@N
>>>
>>>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Robert Houghton <rh...@pivotal.io>.
Please refer to https://issues.apache.org/jira/browse/INFRA-20270

On Fri, May 15, 2020 at 2:58 PM Robert Houghton <rh...@pivotal.io>
wrote:

> Support seems to be unanimously good, and folks voted in this thread. I'm
> going to make the ASF-INFRA request.
>
> On Fri, May 15, 2020 at 11:47 AM Anthony Baker <ba...@vmware.com> wrote:
>
>> Barry and I tossed up a draft PR to fix a problem in session state
>> replication with Tomcat.  If we can get this completed I’d like to include
>> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
>> after 9.0.21.
>>
>> Anthony
>>
>>
>> > On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com> wrote:
>> >
>> > +1
>> >
>> > On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:
>> >
>> >> +1. The more we can automate this types of checks the better.
>> >>
>> >> Thanks,
>> >> Mark
>> >>
>> >>
>> >
>> > --
>> > Ju@N
>>
>>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Robert Houghton <rh...@pivotal.io>.
Support seems to be unanimously good, and folks voted in this thread. I'm
going to make the ASF-INFRA request.

On Fri, May 15, 2020 at 11:47 AM Anthony Baker <ba...@vmware.com> wrote:

> Barry and I tossed up a draft PR to fix a problem in session state
> replication with Tomcat.  If we can get this completed I’d like to include
> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
> after 9.0.21.
>
> Anthony
>
>
> > On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com> wrote:
> >
> > +1
> >
> > On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:
> >
> >> +1. The more we can automate this types of checks the better.
> >>
> >> Thanks,
> >> Mark
> >>
> >>
> >
> > --
> > Ju@N
>
>

Re: 1.13 potential change

Posted by Donal Evans <do...@pivotal.io>.
I'm also bad at using my eyes, apparently. The PR was right there in the
list of open ones. I don't know how I missed it.

On Fri, May 15, 2020 at 4:59 PM Anthony Baker <ba...@vmware.com> wrote:

> https://github.com/apache/geode/pull/5110
>
> Sorry, I’m bad at emailing today.
>
> On May 15, 2020, at 4:28 PM, Donal Evans <doevans@pivotal.io<mailto:
> doevans@pivotal.io>> wrote:
>
> Is there a link to the PR in question? I don't see anything on GitHub.
>
> On Fri, May 15, 2020 at 4:23 PM Anthony Baker <bakera@vmware.com<mailto:
> bakera@vmware.com>> wrote:
>
> Whoops, this got stuck on the wrong thread.  Resending.
>
> We’re continuing to investigate some compatibility issues; there may be
> further changes needed.
>
>
> Anthony
>
>
> On May 15, 2020, at 11:46 AM, Anthony Baker <bakera@vmware.com<mailto:
> bakera@vmware.com>> wrote:
>
> Barry and I tossed up a draft PR to fix a problem in session state
> replication with Tomcat.  If we can get this completed I’d like to include
> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
> after 9.0.21.
>
> Anthony
>
>
> On May 15, 2020, at 1:27 AM, Ju@N <jujoramos@gmail.com<mailto:
> jujoramos@gmail.com>> wrote:
>
> +1
>
> On Thu, 14 May 2020 at 22:19, Mark Hanson <mhanson@pivotal.io<mailto:
> mhanson@pivotal.io>> wrote:
>
> +1. The more we can automate this types of checks the better.
>
> Thanks,
> Mark
>
>
>
> --
> Ju@N
>
>
>
>
>

Re: 1.13 potential change

Posted by Anthony Baker <ba...@vmware.com>.
https://github.com/apache/geode/pull/5110

Sorry, I’m bad at emailing today.

On May 15, 2020, at 4:28 PM, Donal Evans <do...@pivotal.io>> wrote:

Is there a link to the PR in question? I don't see anything on GitHub.

On Fri, May 15, 2020 at 4:23 PM Anthony Baker <ba...@vmware.com>> wrote:

Whoops, this got stuck on the wrong thread.  Resending.

We’re continuing to investigate some compatibility issues; there may be
further changes needed.


Anthony


On May 15, 2020, at 11:46 AM, Anthony Baker <ba...@vmware.com>> wrote:

Barry and I tossed up a draft PR to fix a problem in session state
replication with Tomcat.  If we can get this completed I’d like to include
it with v1.13.0.  I believe our tests will fail with any version of Tomcat
after 9.0.21.

Anthony


On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com>> wrote:

+1

On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io>> wrote:

+1. The more we can automate this types of checks the better.

Thanks,
Mark



--
Ju@N





Re: 1.13 potential change

Posted by Donal Evans <do...@pivotal.io>.
Is there a link to the PR in question? I don't see anything on GitHub.

On Fri, May 15, 2020 at 4:23 PM Anthony Baker <ba...@vmware.com> wrote:

> Whoops, this got stuck on the wrong thread.  Resending.
>
> We’re continuing to investigate some compatibility issues; there may be
> further changes needed.
>
>
> Anthony
>
>
> > On May 15, 2020, at 11:46 AM, Anthony Baker <ba...@vmware.com> wrote:
> >
> > Barry and I tossed up a draft PR to fix a problem in session state
> replication with Tomcat.  If we can get this completed I’d like to include
> it with v1.13.0.  I believe our tests will fail with any version of Tomcat
> after 9.0.21.
> >
> > Anthony
> >
> >
> >> On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com> wrote:
> >>
> >> +1
> >>
> >> On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:
> >>
> >>> +1. The more we can automate this types of checks the better.
> >>>
> >>> Thanks,
> >>> Mark
> >>>
> >>>
> >>
> >> --
> >> Ju@N
> >
>
>

1.13 potential change

Posted by Anthony Baker <ba...@vmware.com>.
Whoops, this got stuck on the wrong thread.  Resending.

We’re continuing to investigate some compatibility issues; there may be further changes needed.


Anthony


> On May 15, 2020, at 11:46 AM, Anthony Baker <ba...@vmware.com> wrote:
> 
> Barry and I tossed up a draft PR to fix a problem in session state replication with Tomcat.  If we can get this completed I’d like to include it with v1.13.0.  I believe our tests will fail with any version of Tomcat after 9.0.21.
> 
> Anthony
> 
> 
>> On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com> wrote:
>> 
>> +1
>> 
>> On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:
>> 
>>> +1. The more we can automate this types of checks the better.
>>> 
>>> Thanks,
>>> Mark
>>> 
>>> 
>> 
>> -- 
>> Ju@N
> 


Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Anthony Baker <ba...@vmware.com>.
Barry and I tossed up a draft PR to fix a problem in session state replication with Tomcat.  If we can get this completed I’d like to include it with v1.13.0.  I believe our tests will fail with any version of Tomcat after 9.0.21.

Anthony


> On May 15, 2020, at 1:27 AM, Ju@N <ju...@gmail.com> wrote:
> 
> +1
> 
> On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:
> 
>> +1. The more we can automate this types of checks the better.
>> 
>> Thanks,
>> Mark
>> 
>> 
> 
> -- 
> Ju@N


Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by "Ju@N" <ju...@gmail.com>.
+1

On Thu, 14 May 2020 at 22:19, Mark Hanson <mh...@pivotal.io> wrote:

> +1. The more we can automate this types of checks the better.
>
> Thanks,
> Mark
>
>

-- 
Ju@N

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Mark Hanson <mh...@pivotal.io>.
+1. The more we can automate this types of checks the better.

Thanks,
Mark


Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Nabarun Nag <nn...@pivotal.io>.
+1

On Thu, May 14, 2020 at 8:54 AM Donal Evans <do...@pivotal.io> wrote:

> Thanks for the explanation, Robert. Also, I realised I never explicitly
> said it in my earlier post, but this is a +1 from me
>
> On Thu, May 14, 2020 at 8:05 AM Joris Melchior <jm...@vmware.com>
> wrote:
>
> > This seems like a good thing to have.
> >
> > +1
> > ________________________________
> > From: Robert Houghton <rh...@pivotal.io>
> > Sent: May 13, 2020 17:32
> > To: dev@geode.apache.org <de...@geode.apache.org>; Mario Ivanac
> > <ma...@est.tech>
> > Subject: [DISCUSS] enable GitHub PR blocking on API breaking changes
> >
> > We have enabled this job on the develop and support 1.13 branches, and it
> > is going well. I would like this to be a blocking job for our pull
> > requests.
> >
> > Are there any issues around this test that we want to address, or reasons
> > to *not* have it be a blocking job in the PR pipeline?
> >
> > To seed the conversation, there is an issue I am working on with @Mario
> > Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.
> >
> > I would like to have a [VOTE] by end of week on this.
> >
>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Donal Evans <do...@pivotal.io>.
Thanks for the explanation, Robert. Also, I realised I never explicitly
said it in my earlier post, but this is a +1 from me

On Thu, May 14, 2020 at 8:05 AM Joris Melchior <jm...@vmware.com> wrote:

> This seems like a good thing to have.
>
> +1
> ________________________________
> From: Robert Houghton <rh...@pivotal.io>
> Sent: May 13, 2020 17:32
> To: dev@geode.apache.org <de...@geode.apache.org>; Mario Ivanac
> <ma...@est.tech>
> Subject: [DISCUSS] enable GitHub PR blocking on API breaking changes
>
> We have enabled this job on the develop and support 1.13 branches, and it
> is going well. I would like this to be a blocking job for our pull
> requests.
>
> Are there any issues around this test that we want to address, or reasons
> to *not* have it be a blocking job in the PR pipeline?
>
> To seed the conversation, there is an issue I am working on with @Mario
> Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.
>
> I would like to have a [VOTE] by end of week on this.
>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Joris Melchior <jm...@vmware.com>.
This seems like a good thing to have.

+1
________________________________
From: Robert Houghton <rh...@pivotal.io>
Sent: May 13, 2020 17:32
To: dev@geode.apache.org <de...@geode.apache.org>; Mario Ivanac <ma...@est.tech>
Subject: [DISCUSS] enable GitHub PR blocking on API breaking changes

We have enabled this job on the develop and support 1.13 branches, and it
is going well. I would like this to be a blocking job for our pull
requests.

Are there any issues around this test that we want to address, or reasons
to *not* have it be a blocking job in the PR pipeline?

To seed the conversation, there is an issue I am working on with @Mario
Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.

I would like to have a [VOTE] by end of week on this.

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Robert Houghton <rh...@pivotal.io>.
@donal There is a violation filter in place for major version changes that
would currently allow any change. That can be narrowed down to only allow
deletion of already-deprecated signatures.

On Wed, May 13, 2020 at 5:18 PM Owen Nichols <on...@pivotal.io> wrote:

> If ApiChecker fails on your PR, and you merge anyway, it will fail on
> develop, so there’s no question it should be a required PR check.
>
> We left some checks optional to avoid blocking merges on unrelated flaky
> failures.  ApiChecker should never be flaky.
>
> If/when Geode decides it’s time for a 2.0 release, which would allow
> breaking backward compatibility, we should be able to provide a rule to
> allow that in the ApiChecker.
>
> > On May 13, 2020, at 3:49 PM, Donal Evans <do...@pivotal.io> wrote:
> >
> > Given that this job takes less than 10 minutes to run, pass or fail, I
> > don't see it adding any additional friction to the PR process in terms of
> > having to wait for things to finish. I am curious if there are any
> > circumstances we could envisage where skipping or bypassing this check
> > would be warranted though, and what the procedure would be in those
> cases.
> > For example, how would it handle the removal of a deprecated method from
> an
> > API? It's not something that happens often, but it will happen eventually
> > (I would hope).
> >
> > On Wed, May 13, 2020 at 2:32 PM Robert Houghton <rh...@pivotal.io>
> > wrote:
> >
> >> We have enabled this job on the develop and support 1.13 branches, and
> it
> >> is going well. I would like this to be a blocking job for our pull
> >> requests.
> >>
> >> Are there any issues around this test that we want to address, or
> reasons
> >> to *not* have it be a blocking job in the PR pipeline?
> >>
> >> To seed the conversation, there is an issue I am working on with @Mario
> >> Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.
> >>
> >> I would like to have a [VOTE] by end of week on this.
> >>
>
>

Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Owen Nichols <on...@pivotal.io>.
If ApiChecker fails on your PR, and you merge anyway, it will fail on develop, so there’s no question it should be a required PR check.

We left some checks optional to avoid blocking merges on unrelated flaky failures.  ApiChecker should never be flaky.

If/when Geode decides it’s time for a 2.0 release, which would allow breaking backward compatibility, we should be able to provide a rule to allow that in the ApiChecker.

> On May 13, 2020, at 3:49 PM, Donal Evans <do...@pivotal.io> wrote:
> 
> Given that this job takes less than 10 minutes to run, pass or fail, I
> don't see it adding any additional friction to the PR process in terms of
> having to wait for things to finish. I am curious if there are any
> circumstances we could envisage where skipping or bypassing this check
> would be warranted though, and what the procedure would be in those cases.
> For example, how would it handle the removal of a deprecated method from an
> API? It's not something that happens often, but it will happen eventually
> (I would hope).
> 
> On Wed, May 13, 2020 at 2:32 PM Robert Houghton <rh...@pivotal.io>
> wrote:
> 
>> We have enabled this job on the develop and support 1.13 branches, and it
>> is going well. I would like this to be a blocking job for our pull
>> requests.
>> 
>> Are there any issues around this test that we want to address, or reasons
>> to *not* have it be a blocking job in the PR pipeline?
>> 
>> To seed the conversation, there is an issue I am working on with @Mario
>> Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.
>> 
>> I would like to have a [VOTE] by end of week on this.
>> 


Re: [DISCUSS] enable GitHub PR blocking on API breaking changes

Posted by Donal Evans <do...@pivotal.io>.
Given that this job takes less than 10 minutes to run, pass or fail, I
don't see it adding any additional friction to the PR process in terms of
having to wait for things to finish. I am curious if there are any
circumstances we could envisage where skipping or bypassing this check
would be warranted though, and what the procedure would be in those cases.
For example, how would it handle the removal of a deprecated method from an
API? It's not something that happens often, but it will happen eventually
(I would hope).

On Wed, May 13, 2020 at 2:32 PM Robert Houghton <rh...@pivotal.io>
wrote:

> We have enabled this job on the develop and support 1.13 branches, and it
> is going well. I would like this to be a blocking job for our pull
> requests.
>
> Are there any issues around this test that we want to address, or reasons
> to *not* have it be a blocking job in the PR pipeline?
>
> To seed the conversation, there is an issue I am working on with @Mario
> Ivanac <ma...@est.tech> regarding exemptions to the Gradle task.
>
> I would like to have a [VOTE] by end of week on this.
>