-
Notifications
You must be signed in to change notification settings - Fork 11
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
ZPM UI overhaul (WIP) #187
Conversation
This is better than having a "false" file (specified as `- ""`). This created an error with a stacktrace if the user tries to do `zpm new`, immediately followed by `zpm bundle`. Now, if the user tries to bundle an "empty" project, they get a more meaningful error message.
The `zapp` variable wasn't being provided to the template rendering, and thus was failing. There was also an issue with `auth_opts` not rendering correctly.
FYI, there are bugs with this during deployment (if there is no UI). I need to do some more testing. |
@@ -316,9 +352,6 @@ def bundle_project(root): | |||
" the zapp.yaml matched anything." | |||
) | |||
|
|||
if not zapp.get('ui'): | |||
_add_ui(tar, zapp) |
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 believe this was the last caller of _add_ui
.
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.
Okay, I'll remove it. Good catch!
For version 1.0 auth, for example, we were expecting the version to be given as '1.0'. I found a real-world case where it was just '1' instead, and it pretty much broke the auth option rendering in the UI template.
@mgeisler I made some updates to this branch, can you please take a look? |
Merged as it already looked good before the final two fixes you made. |
Fixes #109.