-
Notifications
You must be signed in to change notification settings - Fork 3k
fix: add spring-security extension metadata #5738
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for taking care of that, I added a minor comment.
metadata: | ||
keywords: | ||
- "spring-security" | ||
- "spring" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add security
as a keyword too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just force-pushed an amended commit with the security
keyword added. Let's wait for CI.
@aloubyansky @maxandersen any chance we could fail if one extension is missing the metadata? |
We surely can fail if no descriptor is provided. OTOH, I like the idea of not requiring a metadata and generating the basic one as the default. It'd be one of the requirements however to have a detailed enough metadata to be accepted in the platform. |
@aloubyansky any chance it could be optional and we could enforce that in the Quarkus and Platform builds? |
I think so. However it actually has to be reviewed by somebody accepting
the extension, it's not something that can be relied upon blindly.
…On Mon, Nov 25, 2019, 11:33 Guillaume Smet ***@***.***> wrote:
@aloubyansky <https://github.com/aloubyansky> any chance it could be
optional and we could enforce that in the Quarkus and Platform builds?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5738?email_source=notifications&email_token=AACO6M3VF54J73CFKYFUWNDQVOSY7A5CNFSM4JRGP3S2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFB5L6I#issuecomment-558093817>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACO6MZ34NCGUWHNOB7PRZ3QVOSY7ANCNFSM4JRGP3SQ>
.
|
@aloubyansky I think if the file is there, people will review it properly. But I'm not sure they will think of it if it's missing (we have a good example here). |
9942575
to
b5ac769
Compare
we definitely could do a "platform-lint" that could warn/errror on issues; especially something we could enforce in platform and at least in quarkus hosted extensions. |
Merged, thanks! |
I think I missed that when adding the spring-security extension.
WDYT @geoand @michalszynkiewicz @gsmet ?
Related to #5225