Skip to content
This repository was archived by the owner on Dec 17, 2025. It is now read-only.

Update namespace v2#465

Merged
leoluz merged 2 commits intoargoproj:masterfrom
blakepettersson:update-namespace-v2
Nov 3, 2022
Merged

Update namespace v2#465
leoluz merged 2 commits intoargoproj:masterfrom
blakepettersson:update-namespace-v2

Conversation

@blakepettersson
Copy link
Copy Markdown
Member

@blakepettersson blakepettersson commented Sep 22, 2022

Rename WithNamespaceCreation to WithNamespaceModifier, since this
method is also used for modifying existing namespaces. This method
takes a single argument for the actual updating, and unless this method
gets invoked by its caller no updating will take place (fulfilling what
the createNamespace argument used to do).

Within autoCreateNamespace, everywhere where we previously added tasks
we'll now need to check whether the namespace should be created (or
modified), which is now delegated to the appendNsTask and
appendFailedNsTask methods.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 22, 2022

Codecov Report

Base: 55.46% // Head: 55.66% // Increases project coverage by +0.19% 🎉

Coverage data is based on head (48f686d) compared to base (98ccd3d).
Patch coverage: 80.00% of modified lines in pull request are covered.

❗ Current head 48f686d differs from pull request most recent head 57f0dd2. Consider uploading reports for the commit 57f0dd2 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #465      +/-   ##
==========================================
+ Coverage   55.46%   55.66%   +0.19%     
==========================================
  Files          41       41              
  Lines        4504     4513       +9     
==========================================
+ Hits         2498     2512      +14     
+ Misses       1813     1807       -6     
- Partials      193      194       +1     
Impacted Files Coverage Δ
pkg/sync/sync_context.go 73.46% <80.00%> (+0.83%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@leoluz
Copy link
Copy Markdown
Contributor

leoluz commented Sep 28, 2022

@blakepettersson Tks for working on this feature!
Make sure you sign off all your commits. The DCO job in our CI needs to be green before we are able to merge your PR.

@blakepettersson
Copy link
Copy Markdown
Member Author

Make sure you sign off all your commits. The DCO job in our CI needs to be green before we are able to merge your PR.

It's been well overdue for me to add the git hook, now I've done that 😄

…created … (argoproj#455)"

This reverts commit ce2fb70.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
@blakepettersson blakepettersson force-pushed the update-namespace-v2 branch 4 times, most recently from 97ccb81 to 954df91 Compare October 5, 2022 08:42
Copy link
Copy Markdown
Contributor

@leoluz leoluz left a comment

Choose a reason for hiding this comment

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

Added a few more comments.
I think we are close to get this PR ready. Tks!

Rename `WithNamespaceCreation` to `WithNamespaceModifier`, since this
method is also used for modifying existing namespaces. This method
takes a single argument for the actual updating, and unless this method
gets invoked by its caller no updating will take place (fulfilling what
the `createNamespace` argument used to do).

Within `autoCreateNamespace`, everywhere where we previously added tasks
we'll now need to check whether the namespace should be created (or
modified), which is now delegated to the `appendNsTask` and
`appendFailedNsTask` methods.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
8.6% 8.6% Duplication

Copy link
Copy Markdown
Contributor

@leoluz leoluz left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you for the hard work!

@leoluz
Copy link
Copy Markdown
Contributor

leoluz commented Oct 18, 2022

This PR introduces a breaking change in current Argo CD master. To avoid possible build issues this should be merged together with argoproj/argo-cd#10672

@leoluz leoluz merged commit b371e3b into argoproj:master Nov 3, 2022
@blakepettersson blakepettersson deleted the update-namespace-v2 branch November 3, 2022 19:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants