-
Notifications
You must be signed in to change notification settings - Fork 12
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
deploy app from git #510
deploy app from git #510
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
A couple of nitpicks before I get into a code review:
- I would think the radio buttons would make more sense being below the Page Title, from both a visual and accessibility point of view
- I can't think of a reason why these options would be visible when in headless mode, I would suggest hiding those when headless as well
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.
Let me know if there are any questions
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.
Should just be a little more cleanup.
@jbouder made the changes to the Radio buttons, put them below the Deploy titles. |
@aktech made updates to repository config object |
@aktech Ooops! |
@aktech made the Error title more generic, to 'ERROR' |
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 to me. Thanks for working on this. 🎉
Looks like all the tests are failing after merge to main, can you take a look: https://github.com/nebari-dev/jhub-apps/actions/runs/11889273170/job/33132014404 @kildre @jbouder |
Reference Issues or PRs
What does this implement/fix?
Adds functinality to the Deploy App form to pull in an app from a git repo
Put a
x
in the boxes that applyTesting
Documentation
Access-centered content checklist
Text styling
H1
or#
in markdown).Non-text content
Any other comments?