Skip to content
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

#313 baton migration for argocd template changes #317

Merged

Conversation

csun-cpointe
Copy link
Contributor

No description provided.

@csun-cpointe csun-cpointe linked an issue Sep 6, 2024 that may be closed by this pull request
2 tasks
@csun-cpointe csun-cpointe force-pushed the 313-feature-baton-migration-for-argocd-template-changes branch 2 times, most recently from a4d378b to 4372505 Compare September 6, 2024 20:47
continue;
}

if (startSpecConfig && !line.isEmpty() && !line.startsWith(SPEC_INDENTION)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A: Probably better to use the same indentation detection logic as you wrote for the values file migration

Then the template is unchanged

Scenario: A not ArgoCD application template is not impacted by this migration
Given a not application ArgoCD template
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A:

Suggested change
Given a not application ArgoCD template
Given a template that is not an ArgoCD application

- CreateNamespace=true
- ApplyOutOfSyncOnly=true
automated:
prune
Copy link
Contributor

@ewilkins-csi ewilkins-csi Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A: Because this is a yml list, the proper format would be to precede with -. Doesn't really matter much as we're just confirming that the content is preserved.

Edit: sorry got confused with syncOptions, would be prune: "". And the point still doesn't really matter haha

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah this should be prune: true. Let me correct it

Copy link
Contributor

@ewilkins-csi ewilkins-csi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@csun-cpointe csun-cpointe force-pushed the 313-feature-baton-migration-for-argocd-template-changes branch from 4372505 to 617c933 Compare September 6, 2024 22:14
@csun-cpointe csun-cpointe merged commit 0224c2c into dev Sep 6, 2024
@csun-cpointe csun-cpointe deleted the 313-feature-baton-migration-for-argocd-template-changes branch September 6, 2024 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Baton migration for ArgoCD template changes
2 participants