On 03/10/2018 21:49, Nico Williams wrote:
:
A lot of these changes are interrelated. Reviewing them in order of
size might require rebasing our stack of commits, and may not be
entirely possible.
We're extremely familiar with this code as we have been patching the
JGSS stack this way for years (we have developed these patches for JDKs
7, 8, 9, 10, 11, and the current 12 master), and we have been running
this in production (with JDKs 7, 8, and 9, and soon 11)
Just a few high-level points on the patches that you attached:
1. It's important to take sponsor/Reviewer effort into account. I
skimmed through some of the 25 patches and they lack a detailed
description on what the issue is about. JGSS gurus might recognize some
of these issues from the diffs but I suspect you (or Victor) will need
to match the patches to existing issues in JIRA or else get bugs
submitted so that there is a description for each issue in the bug database.
2. I skimmed the patches and didn't see any tests or changes to existing
tests. This may come up in the discussion of each change as the default
position is for all bug fixes should have tests where feasible.
3. I see the patches include at least some API changes so the sponsor
will need to submit a CSR for approval. API changes are only allowed in
feature releases.
-Alan
