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.

Sign up for GitHub

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

Jump to bottom

Supporting regex in config path #636

Merged
merged 15 commits into from
Sep 5, 2023
Merged

Supporting regex in config path #636

merged 15 commits into from
Sep 5, 2023

Conversation

sethiyash
Copy link
Contributor

@sethiyash sethiyash commented Nov 8, 2022 •
edited
Loading

What this PR does / why we need it:

Start supporting regex in config path that will match all the path which adhere to regex

Which issue(s) this PR fixes:

#62

Fixes #

Does this PR introduce a user-facing change?

Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:

@100mik
Copy link
Contributor

100mik commented Nov 14, 2022

Interesting stuff!
Looks like the linter is not happy and there is some commented/dead code, will take a look at this post cleanup 🚀

praveenrewar
praveenrewar reviewed Jan 9, 2023
View reviewed changes
pkg/kapp/resources/mod_path.go Outdated Show resolved Hide resolved
test/e2e/config_test.go Outdated Show resolved Hide resolved
test/e2e/config_test.go Outdated Show resolved Hide resolved
test/e2e/config_test.go Outdated Show resolved Hide resolved
@sethiyash sethiyash force-pushed the regex-config-path branch 2 times, most recently from 7130ae9 to af9158c Compare January 16, 2023 10:22
praveenrewar
praveenrewar reviewed Jan 16, 2023
View reviewed changes
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

Still going through, but submitting for the time being...

pkg/kapp/resources/mod_field_copy.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_path.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_copy.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_copy.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_remove.go Outdated Show resolved Hide resolved
praveenrewar
praveenrewar reviewed Jan 17, 2023
View reviewed changes
pkg/kapp/resources/mod_field_remove.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_copy.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_remove.go Outdated Show resolved Hide resolved
test/e2e/config_test.go Outdated Show resolved Hide resolved
test/e2e/config_test.go Show resolved Hide resolved
kumaritanushree
kumaritanushree reviewed Jan 17, 2023
View reviewed changes
test/e2e/config_test.go Outdated Show resolved Hide resolved
test/e2e/config_test.go Outdated Show resolved Hide resolved
@sethiyash sethiyash force-pushed the regex-config-path branch 2 times, most recently from 80b4696 to e4b8b9a Compare January 20, 2023 13:26
@kumaritanushree
Copy link
Contributor

LGTM

1 similar comment
@rohitagg2020
Copy link
Contributor

LGTM

@sethiyash sethiyash force-pushed the regex-config-path branch 2 times, most recently from 7edb5b2 to ca885e3 Compare March 3, 2023 08:56
@sethiyash sethiyash marked this pull request as draft March 3, 2023 09:17
@sethiyash sethiyash marked this pull request as ready for review March 6, 2023 05:06
@sethiyash
fixing linting issues
36585b9 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
stop overwriting object in copy mod and made changes in remove mod
c75a93b 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
tweaked the logic to resolve issues
4aceebe 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
added e2e testcase with regex in config
a22ddeb 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
made suggested changes on e2e testcases
e9a8451 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
regex as seprate path part
9e45923 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
renamed regexObj to regex
a769621 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
fixing testcase
621e79d 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
added nil check Signed-off-by: Yash Sethiya <[email protected]>
d99b646 
Signed-off-by: sethiyash <[email protected]>
@sethiyash
refactored logic to support regex with indexes
ab33947 
Signed-off-by: sethiyash <[email protected]>
praveenrewar
praveenrewar reviewed Jun 5, 2023
View reviewed changes
pkg/kapp/resources/mod_field_copy.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_copy.go Show resolved Hide resolved
pkg/kapp/resources/mod_field_copy.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_remove.go Outdated Show resolved Hide resolved
pkg/kapp/resources/mod_field_remove.go Outdated Show resolved Hide resolved
@sethiyash sethiyash force-pushed the regex-config-path branch 3 times, most recently from f07be0c to 94650f4 Compare June 6, 2023 10:16
@sethiyash
adopted some nits
566b3f9 
Signed-off-by: sethiyash <[email protected]>
praveenrewar
praveenrewar approved these changes Sep 5, 2023
View reviewed changes
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

LGTM!

@sethiyash sethiyash force-pushed the regex-config-path branch 2 times, most recently from 996d902 to fa112be Compare September 5, 2023 12:01
@sethiyash sethiyash merged commit ec961f8 into develop Sep 5, 2023
5 checks passed
@joaopapereira joaopapereira deleted the regex-config-path branch September 5, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kumaritanushree kumaritanushree kumaritanushree left review comments

@rohitagg2020 rohitagg2020 rohitagg2020 left review comments

@praveenrewar praveenrewar praveenrewar approved these changes

@100mik 100mik Awaiting requested review from 100mik

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sethiyash @100mik @kumaritanushree @rohitagg2020 @praveenrewar @vmwclabot