-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Building my first extension guide #2406
Building my first extension guide #2406
Conversation
@gsmet @ebramirez This PR is clearly a draft but I would appreciate feedbacks on the format before moving forward. Thanks |
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 think it's pretty good. Not too verbose, imo. The request you linked is basically asking for this kind of info, afaiu. Thanks!
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.
So far, so good. It conveys the info in the docs into a simple process easy to follow.
But what I was looking forward is what comes next: What goes where in converting a CDI extension like this into a Quarkus extension.
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.
Nice work. I added a bunch of comments and suggestions.
Whoaa!! Thanks for this complete review. I'll work on the comments ASAP. |
Quick update, just to lett you know that I struggle with time at the moment. I'll work on this ASAP. |
@danielpetisme FWIW, if you need some additional motivation, we have a lot of people asking for it and for the Maven mojo to create an extension :). |
@danielpetisme do you think you'll have time to update this PR or would you mind if we took this over? |
@FroMage Sorry I missed the notification. |
a2cb95a
to
8894499
Compare
I have a bit of time so I can progress on the doc but I still have 2 issues:
|
3c7d9a2
to
8479050
Compare
Folks, after a too long break I hqve continued the writing.
So far, I only covered the very basic concepts but I tried to be as pedagogical as possible. IMHO, the current extension dev doc should continues to be exhaustive and considered as the reference guide. The doc I'm writing is, IMHO, opinionated (I don't go into details when I don't feel the need to) thus more accessible to new comers. I would appreciate an another round of review, 🙏 As you will see in the document there are still sections I want to cover, but IMHO, we could publish a first version as is (even with the TODO mentioned). This would permit to remove me from the critical path and let other people to contribute to the writing (@FroMage asked to take over for instance), plus it's easier to have smaller pieces of doc to review rather than 1K lines). WDYT? |
586f49a
to
68ad62a
Compare
I thought it was helpful to detail how to debug extensions (I had the issue last week). |
Closing due to inactivity. |
Do you think, something is salvageable? Inactivity might not be a problem for a guide. Thoughts? |
It was on-ice for a long time. |
A step by step guide I think is really necessary. I've heard complaints from a few people lately :) |
I do agree the guide does not cover 100% of the extension development. IMHO, the current version could be considered as a chapter1 of a bigger guide. I asked for reviews/comments 2 months ago but the PR has been labeled on-ice (I had the discussion in off with @cescoffier, at the time the extension dev method and tools + the docs was under discussion internally if I'm right) so I froze the contribution. If I can have some guidelines I would be more than happy to update the PR. |
I don't remember plans for it being discussed internally - I've been hunting down most internal Quarkus project conversations, they are now mostly extinct as we want everything to be discussed in the open. Let me reopen this one and add it to the extension doc epics. |
70c3c03
to
7e64d0d
Compare
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 added a bunch of comments.
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 added a bunch of comments.
65f862f
to
d2490dc
Compare
<dependency> | ||
<groupId>io.quarkus</groupId> | ||
<artifactId>quarkus-core-deployment</artifactId> | ||
</dependency> |
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.
BTW, direct dependency on quarkus-core-deployment isn't necessary once quarkus-undertow-deployment has been added (it gets pulled in as a transitive dep instead).
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.
Good point but the core deployment comes from the MOJO it could be misleading for newcomers. Either we let it as is even though is redundant or we explain it in the guide.
T E S T S | ||
------------------------------------------------------- | ||
Running org.acme.quarkus.greeting.deployment.GreetingTest | ||
Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec |
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.
So the test actually didn't run.
@danielpetisme would you like me to take it over? It's been taking quite awhile. I've got some improvements to create-extension on the way #6748 |
48e090c
to
2adcf23
Compare
@aloubyansky I rebuild from scratch the extension, you can find the code at I tried to fix the typos between this guide and the code. Regarding your comment on the extension tests. Anyway, the test is still faillin because I didn't test yet to include the extension into an app. Do you had other errors? |
Thanks for investigating. I haven't done any progress yet. I want to get my create-extension PR in first. I'll be on PTO until Wednesday and will get back to this issue when I'm back. |
2adcf23
to
697628a
Compare
I rebuillt the quickstart @gsmet Would you mind to do your review-sniper thing 😄 please. |
Hey all, this good content is waiting here for ever. At this stage, I'd favor a no/little review and publish, instead of letting it rot? Thouhts? |
697628a
to
ef7dd36
Compare
Agreed. It slipped through the cracks. I rebased, squashed everything and renamed the file to be consistent with our new naming scheme. Will merge as soon as CI is green. |
Too bad we can't see each others in real, I would have make a T-Shirt with It was the oldest open PR on the project 😄 Thank you @gsmet to finish it! |
Well, to my defence, it was still with a WIP title and in a Draft status ;). |
Considering the project has only been public for 14 months, that's good :D |
Thank you all folks, I learned a ton of things writing this documentation! Hope it will help. |
This PR aims to provide a step by step guide about Quarkus extension development as discussed in #1964. I tried to be as pedological as possible by explaining the very basic concepts of Quarkus.