-
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
Dev UI indicate properties set by DevServices #19541
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.
LGTM
what happens if user have yaml configs ? does that "just work" ? |
As I mentioned in the video, I do not think yaml is supported, even in the form editor. I have not tested it, but from the code it does not look like it. I can have a look at that in another PR ? |
makes sense @phillip-kruger - the config viewer supports both so its unfortunate if edit falls apart on that. but sure - separate PR/effort for that makes sense. |
@maxandersen - If you say config viewer do you mean the current form editor ? It might be able to read from Line 60 in 7eb76b4
|
Small nitpick: if the button are round, you need a bit of space between them (i.e. between the cross from the form, then the Dev Services button and then the third one). Other than that, nice improvement! |
Thanks @gsmet ! I'll update with a space |
@gsmet See updated screen ^^^ - Ready for review. |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 9767678
Full information is available in the Build summary check run. Test Failures⚙️ JVM Tests - JDK 11 #📦 extensions/vertx-http/deployment✖
✖
✖
⚙️ JVM Tests - JDK 11 Windows #📦 extensions/vertx-http/deployment✖
✖
✖
⚙️ JVM Tests - JDK 16 #📦 extensions/vertx-http/deployment✖
✖
✖
|
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 9767678
Full information is available in the Build summary check run. Test Failures⚙️ JVM Tests - JDK 11 #📦 extensions/vertx-http/deployment✖
✖
✖
⚙️ JVM Tests - JDK 11 Windows #📦 extensions/vertx-http/deployment✖
✖
✖
⚙️ JVM Tests - JDK 16 #📦 extensions/vertx-http/deployment✖
✖
✖
|
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 have a couple of comments:
- Can we automate the config keys in the extension descriptor somehow? This is something that is easy to miss when updating things.
- What is the use case for copying to a new profile, given that the dev service values are transient and different on each startup?
Let me know what you think. I am looking at the CI failure now. |
Signed-off-by: Phillip Kruger <[email protected]>
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 like a great feature, and blog-worthy, congrats !
Agree with Stuart that it would be nice to automate, there must be a way to detect which config keys apply to which extensions, I think we do something similar during the Quarkus build to generate the all-config page?
Also agree with Philip that the use-case of knowing what properties are the minimum required by to run outside of dev mode is useful when wanting to deploy on staging/prod for the first time.
This config editor originally seemed like a gimmick but now I'm starting to think that between this and the build system that Stuart is discussing on quarkus-dev, we're on to being able to help new users do things that would just be a pain otherwise. Like, this is much simpler than writing IDE plugins for all existing IDEs for a config editor. I don't think we'll want to go all-in and also provide an integrated web IDE in Quarkus for the rest of the code, but since we can already direct from DEV UI to IDEs easily, I only regret that going from the IDE to the DEV UI isn't something we've solved. Like, instead of coding a config editor for Eclipse, it would be super nice if Eclipse would open the config editor page in the DEV UI instead, if it's running…
You know that you can press 'd' in the console to open the dev UI right? |
I guess we can try and automate this later, but I would like to see what @gsmet thinks. |
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.
Works as advertised! Good job!
One thing I'll suggest: would it be possible to move configuration properties that were specifically set to the top (or display them differently)? For example, I set quarkus.application.version=1.0
using the application.properties Online Text Editor but I had a hard time trying to find it when it navigated back to the Config Editor screen
Actually ignore that, just found out that the |
I took the time to have a closer look and it's indeed a nice feature and I think we should merge it. One comment though (but not blocking, we can tweak things later), is it so obvious that the button allows to edit the configuration? I wonder if we should make it a bit more obvious. |
As discussed here #18597 (comment)
This PR adds a way to indicate what configuration is been automatically set by DevServices.
It also give an easy way to copy this to
application.properties
for%test
and%prod
.This also added a way to narrow the config screen to configuration of a certain extension and added a basic raw editor.
For now this is a draft to get some feedback.
See this video for a walkthrough:
simplescreenrecorder-2021-08-20_15.29.43.mp4
Signed-off-by: Phillip Kruger [email protected]