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

More descriptive error messages for unsupported Jandex storage format versions #19876

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Sep 2, 2021

No description provided.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 2, 2021

I believe it would be possible to handle an unsupported version error similarly to "too old version" error and reindex on the fly, but it feels weird. So for now, I went with this conservative approach of just improving the error message.

@gastaldi
Copy link
Contributor

gastaldi commented Sep 2, 2021

Isn't the error message already improved in jandex? I think it's fine to wait for 2.4.1 instead

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Looks good!

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 2, 2021

Right, Jandex has an improved error message now. But Jandex's IndexReader doesn't know the problematic file, so can't show it in the error message -- which is what this PR does.

@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 2, 2021
@gastaldi gastaldi merged commit 96c596b into quarkusio:main Sep 2, 2021
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Sep 2, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 2, 2021
@Ladicek Ladicek deleted the better-jandex-error branch September 3, 2021 07:34
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