Skip to content

https://github.com/oracle/graal/issues/11050 - #869

Open
junietrytwo wants to merge 1 commit into
graalvm:masterfrom
junietrytwo:master
Open

https://github.com/oracle/graal/issues/11050#869
junietrytwo wants to merge 1 commit into
graalvm:masterfrom
junietrytwo:master

Conversation

@junietrytwo

Copy link
Copy Markdown

@oracle-contributor-agreement

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement Bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Apr 7, 2026
@junietrytwo

Copy link
Copy Markdown
Author

Hello team, I also find this feature request interesting.

Could you please help review my code?

Thank you

@vjovanov
vjovanov requested review from pdekanov and vjovanov July 20, 2026 11:48
@vjovanov

Copy link
Copy Markdown
Member

This looks good at first. Can you please complete the PR title and description. Explain how the new feature is working, what is changed, is it enabled by default, etc.

We also need to rebase this PR as we are now spec-driven.

@pdekanov

Copy link
Copy Markdown

Please improve the PR title and add a short description of the change.
You can take a look at any of the open PRs as reference.

return stream
.filter(Files::isRegularFile)
.filter(NativeBuildReportMojo::isHtmlFile)
.max(Comparator.<Path>comparingInt(path -> buildReportScore(path, imageName))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we filler out candidates whose score is zero?
Any unrelated HTML file in target/ is selected when no actual build report exists, so the Maven site could list the wrong page instead of none.

}

@Override
public String getOutputName() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a Maven report path rather than a filesystem path, should this use / instead of File.separator for Windows compatibility?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, as this uses Reporting API 4, it may also be preferable to implement getOutputPath (since getOutputName is deprecated?).

@vjovanov

Copy link
Copy Markdown
Member

@junietrytwo any update on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Required At least one contributor does not have an approved Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants