On Wed, 7 Jul 2021 19:02:04 GMT, Erik Joelsson <er...@openjdk.org> wrote:

> This patch adds copying of element-list files generated for jdk.javadoc to 
> the interim langtools version of jdk.javadoc. I'm also refactoring the 
> jdk.javadoc/Gendata.gmk file a bit to better adhere to current build infra 
> standards as this was missed in the original review of this file. Rebuilding 
> should now work better with the various clean targets and any problems with 
> the generation tool will be captured in separate log files, along with the 
> command lines.
> 
> @hns Can you verify that this solves the problem you reported?

Overall it looks good. Thanks for cleaning up the bad code that inadvertently 
slipped in.

One final remark regarding the marker files `_element_lists`. I've tried to use 
the naming pattern `_<identifier>.marker` for new marker files, to give them a 
clear file type suffix. Converting all old names is a slow process, but I'd 
appreciate it if new files follow this pattern.

make/modules/jdk.javadoc/Gendata.gmk line 84:

> 82:           $(CT_DATA_DESCRIPTION) \
> 83:           $(ELEMENT_LISTS_DIR) \
> 84:           11 \

I'm not sure why JDK 11 is chosen as the starting point, but I suspect it might 
change in the future. Perhaps move this to a variable instead of hardcoding it 
into the command line, so it's more visible?

make/modules/jdk.javadoc/Gendata.gmk line 107:

> 105:  $(RM) -r $(INTERIM_ELEMENT_LISTS_DIR)
> 106:  $(MKDIR) -p $(INTERIM_ELEMENT_LISTS_DIR)
> 107:  $(CP) -R $(ELEMENT_LISTS_DIR)/* $(INTERIM_ELEMENT_LISTS_DIR)/

If you instead do 

$(call MakeDir $(INTERIM_ELEMENT_LISTS_DIR))
$(RM) -r $(INTERIM_ELEMENT_LISTS_DIR)/*


we can save a makedir on rebuilds.

-------------

Changes requested by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/227

Reply via email to