Hi,
I would leave the test and code as is for the purposes of 8133273
and leave it to later to resolve 8167222. There is no compelling need
to change it.
$.02, Roger
On 10/5/2016 4:45 PM, Stuart Marks wrote:
Stephen,
Thanks for the quick followup clarifications. I'm not entirely sure
how you'd like to proceed; see discussion below.
Jonathan, also see below.
On 10/5/16 9:07 AM, Jonathan Bluett-Duncan wrote:
Stuart, thank you very much for your continued review of this
changeset, and for
finding those usages of CookieManager::get in Grepcode for me. I've
applied the
comment you suggested for ModuleFinder and I've also fixed the
NetscapeCookieStore typo.
Great!
Stephen, thank you for getting back about DateTimeFormatter. It's not
clear to
me what should be done with
TCKDateTimeFormatter::test_resolverFields_listOfOneNull in this case.
Do I
delete it; or do I change it to test that a null TemporalField param
causes a
NullPointerException to be thrown; or do I do something else? May I
have your
continued thoughts on this?
OK, this is kind of subtle. This is a TCK (conformance) test, so it
probably cannot simply be removed; it may need a specification change
to clarify this case. I've filed JDK-8167222 to cover these issues.
I've made a note in this bug regarding the potential change in
DateTimeFormatter.withResolverFields() to use Set.of().
(There's an additional wrinkle with Set.of() aside from rejecting
nulls; it also rejects duplicates by throwing IAE.)
In any case that code can't be changed to use Set.of() until the
test/spec issue is resolved, so for the purposes of this changeset,
I'd suggest simply removing the withResolverFields() comment from the
webrev. We can revisit this during or after the resolution of
JDK-8167222.
I think this clears all the issues, so you can probably go ahead and
work with Patrick to update the webrev. And Patrick, thanks once again
for hosting the webrev!
s'marks
