Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

237: Removing hardcoded namespace in safe app code #258

Merged
merged 2 commits into from
Sep 18, 2023
Merged

237: Removing hardcoded namespace in safe app code #258

merged 2 commits into from
Sep 18, 2023

Conversation

abhishek44sharma
Copy link
Contributor

No description provided.

v0lkan
v0lkan requested changes Sep 16, 2023
View reviewed changes
Copy link
Contributor

@v0lkan v0lkan left a comment

Choose a reason for hiding this comment

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

One tiny variable rename to VSECM_SYSTEM_NAMESPACE and we are good.

app/safe/internal/bootstrap/k8s.go Outdated Show resolved Hide resolved
v0lkan
v0lkan requested changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@v0lkan v0lkan left a comment

Choose a reason for hiding this comment

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

One missing end of line.

And also realized we have inconsistency in the docs after this change; it needs to be addressed too.

core/env/init.go Outdated Show resolved Hide resolved
app/safe/internal/bootstrap/k8s.go Show resolved Hide resolved
@abhishek44sharma
237: Removing hardcoded namespace in safe app code
9c6f86e 
Signed-off-by: Abhishek Sharma <abhishek44sharma@gmail.com>
@abhishek44sharma
Copy link
Contributor Author

One missing end of line.

And also realized we have inconsistency in the docs after this change; it needs to be addressed too.

Updated!! 👍

@abhishek44sharma
237: Fixing review comment
b0ce63d 
Signed-off-by: Abhishek Sharma <abhishek44sharma@gmail.com>
v0lkan
v0lkan approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@v0lkan v0lkan left a comment

Choose a reason for hiding this comment

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

LGTM.

@v0lkan v0lkan merged commit 60352c6 into main Sep 18, 2023
@v0lkan v0lkan deleted the topic/abhishek/237 branch September 18, 2023 21:57
@abhishek44sharma abhishek44sharma mentioned this pull request Sep 19, 2023
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@v0lkan v0lkan v0lkan approved these changes

@RamXX RamXX Awaiting requested review from RamXX

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@abhishek44sharma @v0lkan