On Thu, Oct 04, 2018 at 11:19:06AM +0100, Alan Bateman wrote: > 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.
We've pointed out that we need bugs filed. We'll work with Weijun to get them filed when he's back from vacation. > 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. That's an existing problem in the codebase. The tests are very limited as it is, and testing GSS/Kerberos requires mocking up a KDC, which requires more complex test infrastructure. > 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. That's fine. Nico --
