On Thu, 6 Jun 2024 10:42:20 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix default description of keep-packaged-modules > > I've read through all src changes. I think Sundar is looking at the code > changes too. > > The overall design seems solid. I know it took a long time to get there but > this is nature of a feature like this. At this point I regret that there > isn't a JEP. A JEP would have captured the motivation, outlined the design, > the reasoning for the restrictions, and document the design > choices/directions that have been prototyped. If there isn't a JEP then I > suppose it can come later if the feature is progressed and there is > discussion about making it the default rather than opt-in at build configure > time. > > As cleanup, I think we will need to bring some consistency to the terminology > and phrasing in documentation, code and comments. Right now there is > "run-time linking", "linkable run-time", "run-time linkable JDK image", > "linkable jimage". > > Also as cleanup, I think the code needs more comments. There is no JEP right > now with an authoritive description of the feature so anyone maintaining this > code will have to figure out a lot of details. It feels like there should be > somehting that documents the effect of --enable-runtime-link-image, how the > diffs are generated and saved, and how they are used by jlink. There is also > a lot of new code in ImageFileCreator and JlinkTask that is asking for some > method descriptions so that anyone touching this code can quickly understand > what these methods are doing. I don't want to get into code style issues now > but I think it would be helpful for future maintainers to avoid more of the > overfly long lines if possible (some of them are 150, 160, 170+ and really > hard to look at code side-by-side). > @AlanBateman Sure, I appreciate the feedback. Do I understand it correctly > that this won't get into JDK 23 then? If so, perhaps the better way would be > to draft a JEP for JDK 24 and get that proposed early. What I'd like to avoid > is for this change to don't see much movement for a long time between now and > RDP 1 of JDK 24 and have a similar crunch when JDK 24 is close to forking. > You mention clean-up a lot. Is that suggesting it _can_ go into JDK 23 and > clean-up in ramp-down? I'm confused... > > Some clarity on how to best approach getting this integrated that would be > acceptable for all involved would be appreciated. Thanks! The fork for JDK 23 is today so if I was running with this feature then I wouldn't integrate it today. If you are willing to put the time into writing a JEP then I think it's the right thing to do. We should probably have brought this up long before now. I'm happy to help review iterations. There is a lot to write down and this will be very valuable for future phases of this work. I assume future phases. We agreed some restrictions to reduce complexity for this first phase. Future phases may remove these and maybe there is confidence in the future to make it default. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2152576950
