-
Notifications
You must be signed in to change notification settings - Fork 6.6k
feat: support creating multi-source applications in New App panel #21005
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
base: master
Are you sure you want to change the base?
Conversation
❗ Preview Environment undeploy from Bunnyshell failedSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
f5cc83b to
99bd9da
Compare
andrii-korotkov-verkada
left a comment
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.
Please, fix the tests
99bd9da to
8dc15a0
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.
Can you fix the Lint issues and DCO check?
-
You can fix the lint issues by running the command
golangci-lint run --timeout 10m --fix -
If you click on “Details” next to the failed check for DCO, you’ll find instructions how to solve that.
Also, it can be helpful if you can paste a After section in the PR description.
7c9e37d to
7968a5f
Compare
…ix lint prettier errors Signed-off-by: Dave Canton <[email protected]>
7968a5f to
56f1d0d
Compare
Signed-off-by: Dave Canton <[email protected]>
|
Hi @nitishfy thanks a lot for the tip! I have been struggling with the UI lint and its requirements (spacing, line breaks, etc), hence the number of times I have rebased and repushed the same commit (btw sorry if you guys received a flood of notifications in the process @nitishfy @andrii-korotkov-verkada). The UI tests now pass. I have also included source screenshoots in the PR description to show how the create new app panel looks like -- I hope that is what you meant with an After session, @nitishfy? Thanks for the comments! :) |
You are good.
It may be better to not force push to be able to see your incremental changes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #21005 +/- ##
==========================================
- Coverage 55.03% 54.97% -0.07%
==========================================
Files 324 324
Lines 55466 55466
==========================================
- Hits 30526 30492 -34
- Misses 22330 22350 +20
- Partials 2610 2624 +14 ☔ View full report in Codecov by Sentry. |
keithchong
left a comment
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.
Could you rebase?
Also, try reconciling or synchronizing the changes made in the yaml editor with what is on the form, and vice versa.
Are you planning to make the source sections collapsible? (It could be a follow-on PR). That way it'll be consistent with what we already have.
| @@ -0,0 +1,285 @@ | |||
| import {AutocompleteField, DataLoader, DropDownMenu} from 'argo-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.
Did you take this source from
argo-cd/ui/src/app/applications/components/application-parameters/source-panel.tsx
Line 12 in 8f0d3d0
| // This is similar to what is in application-create-panel.tsx. If the create panel |
keithchong
left a comment
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.
Also, the source parameters section should dynamically update based on the app source type (eg. helm or kustomize) that we detect.
|
Hi @dvcanton This PR is useful to us, Could you please resolve it and merge this changes. |
Implements #20964 based on Proposal #17108.
Depends on a fix #581 in the FormField component.
Side note: that is my first contribution to ArgoCD, therefore I am open to improvement ideas, suggestions and discussion points on this PR.
After:
The button Add Source places a new
SourcePanelcomponent and increase a source counter.Checklist: