On Fri, 14 Apr 2023 05:07:18 GMT, Bradford Wetmore <wetm...@openjdk.org> wrote:
>> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address review feedback > > I'm coming to this late, but what is the breadth of the specs you're trying > to call out? Where did you obtain this list? Are all of these changes > coming from existing mentions in the current APIs, and you're just adding a > `@spec` in various places? Or are you trying to be complete, or just list a > representative sample? In part 1, I saw you moved some of the spec mentions > to be in a `@spec`, but in this PR, you're adding specs in the APIs. > > In many of our APIs, we mention things "...such as...RFC 2246...", but we > make no effort to be complete by providing a list of specs. > > For example: > > SSLEngine.java: only TLSv1.0 was mentioned, but there's also > SSLv3/1.1/1.2/1.3, and DTLS 1.0/1.2. > > SSLSocket.java: your change only lists 7301, but not 2246. But same issue > as SSLEngine, there are others specs this also applies to. > > java.security.Key.java: RFC 5280 was the only spec called out. There are > many other Key types. > > SecureRandom: RFC 4086 was called out. There are others. > > If you want to mention a bunch of the security specs, I think we need to > better understand the scope of what you're trying to do, and how this will be > kept in sync with Chapter 4 of the Security Documentation (Providers): which > also could use some updates-e.g. TLSv1.x RFCs, but that is another RFE for > another day. @bradfordwetmore > I'm coming to this late, but what is the breadth of the specs you're trying > to call out? This is a mostly automated update based on existing references to well-known specifications. * **Mostly automated** means it was done by a custom utility program, but I hand-edited the output to improve line-wrapping, etc. * **Existing references** means found in `<a href=...>` in the same doc comment to which the `@spec` is added, where that HTML link may appear in either narrative text or in a `@see` tag * **Well-known specifications** means that the utility was looking for well-known hosts/urls. The list of the hosts that was used is `ietf.org`, `unicode.org`, `w3.org`, `iana.org`, `iso.org`, and references to the sibling `specs` directory that exists beside the main `api` directory. While those are the hosts, the pattern matching was more specific to singleton URLs or groups of URLs at each site. The update uses "normative"/"preferred" URLs for each spec, to avoid the variety of different URLs used for some of the specs, reflecting _different_ hosts and/or paths for the _same_ specification. The goal is just to add these `@spec` tags without any semantic change to the specification. As such, no CSR is required for this work. If there are additional `@spec` tags that should be added for existing references, I can do that here in this PR, or in followup work. It has already been noted that I missed the references to `nist.gov`. If there are additional specifications that should be included (that are not currently mentioned) that is a task for the relevant developers in the relevant component teams. I would expect any such work to be done separately, later. It is also a future-goal to clean up the existing `href` references to the same general form of URL as given in the `@spec` tag, thus normalizing the host and path that we use for each specification (or the root of a multi-page specification.). The effect of the `@spec` tags is to list the specifications for a declaration in a list headed `External Specifications`, similar to the existing `See Also` list. Eventually, we will enable a new summary page headed `External Specifications` that lists all the specs listed in `@spec` tags, providing links to where the specs are mentioned. That page is currently temporarily disabled until we have a more complete set of `@spec` tags in most or all modules. Using the canonical form of spec URLs and titles in `@spec` tags, and having the cross-referenced list on that new `External Specifications` page should make it easier to find and maintain these references going forward. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13336#issuecomment-1509185926
