Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Description for Feature classes #27137

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

gastaldi
Copy link
Contributor

@gastaldi gastaldi commented Aug 4, 2022

Follow up on #26943

This produces an output during the GraalVM compilation explaining what the Feature does and allows you to browse to the existing source code:

Screen.Recording.2022-08-04.at.11.40.57.mov

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 4, 2022

/cc @Sanne, @gsmet, @yrodiere

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

This is nice, but shouldn't the links be something more stable? These will break as soon as we change the directory structure in our repo, and that kind of change has been discussed recently.

Maybe a link to the javadoc (which would only depend on the package/class name) would be safer? I couldn't find where the javadoc of Quarkus is published, though...

@gastaldi
Copy link
Contributor Author

gastaldi commented Aug 4, 2022

This is nice, but shouldn't the links be something more stable? These will break as soon as we change the directory structure in our repo, and that kind of change has been discussed recently.

We could point to a tagged version instead (by having the URL changed during the build) but I am not sure it's worth the effort. We can start by providing only the descriptions now.

@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Aug 5, 2022

Very interesting!

But I totally agree with @yrodiere that we should add something more stable as the URL.

@gastaldi gastaldi force-pushed the feature_description_url branch from 176b810 to 08a6401 Compare August 5, 2022 18:22
@gastaldi gastaldi changed the title Add URL and Description for Feature classes Add Description for Feature classes Aug 5, 2022
@gastaldi
Copy link
Contributor Author

gastaldi commented Aug 5, 2022

I reworked the PR to provide only the description for now, until we figure out a stable URL to use

@gastaldi gastaldi requested a review from yrodiere August 5, 2022 18:23
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Comment on lines +18 to +19
InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(META_INF_QUARKUS_NATIVE_RESOURCES_TXT);
if (resourceAsStream != null) {
Copy link
Member

Choose a reason for hiding this comment

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

I guess you're also fixing a bug here?

That was unexpected, but ok.

@yrodiere yrodiere merged commit 46ac9fe into quarkusio:main Aug 8, 2022
@quarkus-bot quarkus-bot bot added this to the 2.12 - main milestone Aug 8, 2022
@gastaldi gastaldi deleted the feature_description_url branch August 8, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants