On Mon, 26 Jun 2023 15:40:39 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
>> The API changes look great. I like the instance usages of ClassFile, and how >> CF is now used in a centralized fashion to double down as a cache/option >> store (and entry point for transformation). Seems like a step in a good >> direction, and make the API feel a little more cohesive. >> >> A small subjective comment - I'm not too fond of using "context" as a >> variable/accessor name for all the places where we use a classfile. E.g. if >> classfile is the context, perhaps these variable/accessor should just be >> named "classfile" ? > >> The API changes look great. I like the instance usages of ClassFile, and how >> CF is now used in a centralized fashion to double down as a cache/option >> store (and entry point for transformation). Seems like a step in a good >> direction, and make the API feel a little more cohesive. >> >> A small subjective comment - I'm not too fond of using "context" as a >> variable/accessor name for all the places where we use a classfile. E.g. if >> classfile is the context, perhaps these variable/accessor should just be >> named "classfile" ? > > Also, another observation: the number of updates snippets in javadoc seems > rather low for something that touches the main entry point by which client > interacts with classfiles. This seems a sign of perhaps not having many end > to end examples in the API javadoc? @mcimadamore thank you for review! Description of `Classfile` instances as "context" is intentional compromise between discoverability (as originally proposed `Classfile.Context` didn't work very well) and clarity of the function (as `Classfile` instances may be simply confused with `ClassModel` instances). ------------- PR Comment: https://git.openjdk.org/jdk/pull/14180#issuecomment-1607827611
