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

Fix wrong extension reference on WebauthN documentation #29688

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

pjgg
Copy link
Contributor

@pjgg pjgg commented Dec 5, 2022

Fix wrong documentation reference. test-security-webauthn is a helper more than a extension.
Related to: #29662 fyi @gsmet

:create-app-extensions: security-webauthn,reactive-pg-client,resteasy-reactive,hibernate-reactive-panache,test-security-webauthn
:create-app-extensions: security-webauthn,reactive-pg-client,resteasy-reactive,hibernate-reactive-panache
Copy link
Member

Choose a reason for hiding this comment

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

Have you tested if this actually adds the library or if it ignores it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is throwing an error!

quarkus create app org.acme:security-webauthn-quickstart \
    --extension='security-webauthn,reactive-pg-client,resteasy-reactive,hibernate-reactive-panache,test-security-webauthn' \
    --no-code
Looking for the newly published extensions in registry.quarkus.io
[ERROR] ❗  Cannot find a dependency matching 'test-security-webauthn', maybe a typo?
[ERROR] ❗  Unable to create project: Failed to create project because of invalid extensions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same if you try to add the library later on:

quarkus extension add 'test-security-webauthn'
[ERROR] ❗  Nothing installed because keyword(s) 'test-security-webauthn' were not matched in the catalog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but... I like your idea of ignore those dependencies that are not in the catalog!. Maybe if an extension is not in the catalog we could just write a warning message but create the app with those deps that are right.

Copy link
Member

Choose a reason for hiding this comment

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

Could you try adding quarkus-test-security-webauthn instead?

I think it's better to keep it an error so if not working at all, I will merge your patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you try adding quarkus-test-security-webauthn instead?

Same issue with quarkus-test-security-webauthn

@pjgg pjgg requested a review from gsmet December 5, 2022 17:20
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.

TBH, even if it works, I think it's better to remove it so let's merge your patch.

@gsmet gsmet merged commit d09feb0 into quarkusio:main Dec 6, 2022
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 6, 2022
@gsmet
Copy link
Member

gsmet commented Dec 6, 2022

Thanks!

@gsmet gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 6, 2022
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.

2 participants