Skip to content

Conversation

Eng-Fouad
Copy link
Contributor

@Eng-Fouad Eng-Fouad commented Jul 7, 2025

These annotations are used to register resources in native image.

This comment has been minimized.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from 48106e7 to 8d86846 Compare July 7, 2025 06:15

This comment has been minimized.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from 8d86846 to fedb4ff Compare July 7, 2025 06:23
@geoand geoand requested a review from zakkak July 7, 2025 06:24

This comment has been minimized.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch 2 times, most recently from 068369d to 2a6abe4 Compare July 7, 2025 06:32

This comment has been minimized.

This comment has been minimized.

Copy link

github-actions bot commented Jul 7, 2025

🙈 The PR is closed and the preview is expired.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from 2a6abe4 to 713b26f Compare July 7, 2025 07:33

This comment has been minimized.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from 713b26f to c6ad739 Compare July 7, 2025 07:41
@gsmet
Copy link
Member

gsmet commented Jul 7, 2025

Hey @Eng-Fouad , given you contribute quite often, you should have a look at some aliases I created here:

https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md#using-aliases

I usually use build-fast always to check I haven't broken anything inadvertently but format can be useful to fix the formatting quickly.

This comment has been minimized.

@Eng-Fouad

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

@Eng-Fouad Eng-Fouad changed the title Introduce @RegisterResourceBundle and @RegisterResources Introduce @RegisterResourceBundle and @RegisterResources Jul 28, 2025
@Eng-Fouad
Copy link
Contributor Author

@zakkak Could you please review this PR. Thanks.

Copy link
Contributor

@zakkak zakkak left a comment

Choose a reason for hiding this comment

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

Hi @Eng-Fouad, thank you for your contribution.
Please see my comments.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from c6ad739 to f8ca720 Compare August 4, 2025 16:33
@Eng-Fouad Eng-Fouad requested a review from zakkak August 4, 2025 16:34
@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from f8ca720 to 2b4342a Compare August 4, 2025 16:35
@Eng-Fouad
Copy link
Contributor Author

Hi @Eng-Fouad, thank you for your contribution. Please see my comments.

Updated according to your comments. Thanks.

This comment has been minimized.

This comment has been minimized.

@Eng-Fouad
Copy link
Contributor Author

This is weird. I have a newline at the end of each file locally, but it got dropped on pushing to GitHub.

Screenshot 2025-08-05 191259

@Eng-Fouad
Copy link
Contributor Author

This is weird. I have a newline at the end of each file locally, but it got dropped on pushing to GitHub.

Screenshot 2025-08-05 191259

Well. Github just never show the newline at end of the file on preview:

1 2

@zakkak Could you please review this PR again? Thanks :)

@geoand
Copy link
Contributor

geoand commented Aug 7, 2025

Any chance we could introduce these into one of our native tests?

@Eng-Fouad
Copy link
Contributor Author

Any chance we could introduce these into one of our native tests?

Currently there are no tests for @RegisterForReflection/@RegisterForProxy either. Maybe tests could be added to the 4 annotations in a separate PR?

@geoand
Copy link
Contributor

geoand commented Aug 8, 2025

I would much prefer to have this PR add something in one of our integration tests - I assume it won't be too hard.

For @RegisterForProxy, I agree we can add something in a follow-up.

@Eng-Fouad
Copy link
Contributor Author

Eng-Fouad commented Aug 8, 2025

I would much prefer to have this PR add something in one of our integration tests - I assume it won't be too hard.

For @RegisterForProxy, I agree we can add something in a follow-up.

Can you please give me an example for native tests in integration-tests that I can copy from? Thanks.

@geoand
Copy link
Contributor

geoand commented Aug 8, 2025

What I would do is look at integration-tests/main and check if any resources or bundles are being loaded there.
If they are, I would remove the custom configuration that makes them work and replace it with the new annotation.
If they aren't I would just add a new HTTP endpoint that reads some new resource and bundle.

zakkak
zakkak previously requested changes Aug 8, 2025
Copy link
Contributor

@zakkak zakkak 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 @Eng-Fouad.

Waiting for the integration tests requested by @geoand as well before merging.

@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch 4 times, most recently from c5e64f8 to 4d87238 Compare August 8, 2025 20:25
@Eng-Fouad Eng-Fouad force-pushed the native-resources-annotations branch from 4d87238 to 860f3e9 Compare August 8, 2025 20:54
@quarkus-bot quarkus-bot bot added the area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure label Aug 8, 2025
Copy link

quarkus-bot bot commented Aug 8, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 860f3e9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@Eng-Fouad
Copy link
Contributor Author

@geoand @zakkak Integration tests have been added.

Copy link

quarkus-bot bot commented Aug 9, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 860f3e9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gsmet gsmet dismissed zakkak’s stale review August 11, 2025 10:30

IT has been added per request.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Approving as @zakkak was happy with it and an IT has been added.

See #48805 (review) .

@gsmet gsmet merged commit f530bd5 into quarkusio:main Aug 11, 2025
60 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.26 - main milestone Aug 11, 2025
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Aug 11, 2025
@Eng-Fouad Eng-Fouad deleted the native-resources-annotations branch August 11, 2025 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure kind/enhancement New feature or request triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce @RegisterResourceBundle and @RegisterResources to register resources in native image
4 participants