JB,

Yes, I agree it's a PITA to have to re-run the release and delay it so I am
sorry about that (I've had to do it before and it is annoying for sure). In
this case I do think it's a worthwhile thing to do as tests are important.
Going forward I will try and keep more up with some of the PRs (especially
the big ones like the JMS 2.0 stuff) so things can be discussed and caught
earlier.

Also, typically the reviewer (or reviewers) of a pull request would be the
first person to try and catch things like this. To make it easier and more
consistent maybe we should make a checklist/guide that reviewers can use to
do a sanity check? Some things can be checked automatically (license
headers, tests running, etc) but some stuff will need to be done manually.

Besides obviously checking that the PR itself works as designed and doesn't
have any major issues (like thread safety) some other stuff to check off
the top of my head:

1) Make sure all PRs have a matching and well written Jira describing the
issue and a good commit message that references the Jira.
2) Meaningful unit tests written (except for trivial stuff)
3) Verify any comments/documentation added makes sense and is helpful. If
not added but should be then request documentation be added.
4) The build and tests pass on Jenkins (and run the rat plugin to check for
license files, etc)
5) Check code formatting (not weird spacing, it fits in with the existing
coding style and conventions, etc).
6) Make sure it won't break existing users and is backwards compatible
(unless it's a major version and that's the intent, etc)

I also usually like to make sure commits are squashed into 1 and rebased
but that isn't strictly necessary of course. Anyways, there's probably more
things but that would be a good start.

Chris

On Fri, Feb 4, 2022 at 12:26 AM JB Onofré <j...@nanthrax.net> wrote:

> Hi guys
>
> Unfortunately we have a request to cancel 5.16.4 vote to add tests on some
> new features.
>
> It means that it will require some time and new 5.16.4 release. It’s a
> pita to see this request (fair by the way) is happening during vote and not
> before when PRs were there.
>
> So, we will have delay for 5.16.4 but also for 5.17.0.
>
> I do my best to move forward both 5.16.4 and 5.17.0 in parallel but 5.17.0
> vote won’t happen before end of the next week at least.
>
> Regards
> JB
>
> > Le 31 janv. 2022 à 15:36, Matt Pavlovich <mattr...@gmail.com> a écrit :
> >
> > Thanks, JB. I reviewed JIRA and everything I had earmarked is good to
> go for 5.16.4.
> >
> > A lot of nice fixes in this one.
> >
> > -Matt
> >
> >
> >> On Jan 31, 2022, at 1:57 AM, Jean-Baptiste Onofre <j...@nanthrax.net>
> wrote:
> >>
> >> Hi guys,
> >>
> >> I finalize the preparation of 5.16.4 during the week end.
> >>
> >> Just waiting for reporter comment regarding AMQ-8451 but I will
> probably postpone this one to 5.16.5 and cut 5.16.4 release.
> >>
> >> So 5.16.4 will be on vote today.
> >>
> >> Regards
> >> JB
> >>
> >>>> Le 2 janv. 2022 à 07:38, Jean-Baptiste Onofre <j...@nanthrax.net> a
> écrit :
> >>>
> >>> Hi guys,
> >>>
> >>> Just to let you know that I identified a regression in ActiveMQ:
> >>>
> >>> https://issues.apache.org/jira/browse/AMQ-8445
> >>>
> >>> It has been introduced by commit
> 34c4e186fe3d71c82866e89afd2706a3619ca2b4 trying to fix AMQ-8275 (related to
> JDK16+ support in SslTransport).
> >>>
> >>> I’m working on a fix about that. Then I will move forward on 5.16.4
> release first, then 5.17.0.
> >>>
> >>> I will keep you posted soon.
> >>>
> >>> Regards
> >>> JB
> >>
> >
>
>

Reply via email to