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

Create new config object from legacy flags and config #2061

Merged
merged 9 commits into from
Jun 26, 2024

Conversation

ashmeenkaur
Copy link
Collaborator

Description

Create new config object from legacy flags and legacy mount config.

Link to the issue in case of a bug fix.

NA

Testing details

  1. Manual - NA
  2. Unit tests - Added
  3. Integration tests - NA

@ashmeenkaur ashmeenkaur added the execute-integration-tests Run only integration tests label Jun 24, 2024
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

Attention: Patch coverage is 93.68421% with 6 lines in your changes missing coverage. Please review.

Project coverage is 72.12%. Comparing base (2450080) to head (4d3ad08).
Report is 11 commits behind head on CLI_config_parity_release.

Files with missing lines Patch % Lines
cmd/legacy_param_mapper.go 93.68% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           CLI_config_parity_release    #2061      +/-   ##
=============================================================
+ Coverage                      71.96%   72.12%   +0.16%     
=============================================================
  Files                             99      100       +1     
  Lines                          10870    10965      +95     
=============================================================
+ Hits                            7823     7909      +86     
- Misses                          2715     2720       +5     
- Partials                         332      336       +4     
Flag Coverage Δ
unittests 72.12% <93.68%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ashmeenkaur ashmeenkaur changed the base branch from master to dont_use_typecasted_strings_in_mount_config June 24, 2024 17:04
@ashmeenkaur ashmeenkaur force-pushed the dont_use_typecasted_strings_in_mount_config branch from 8b3b628 to 6a77191 Compare June 24, 2024 17:08
@ashmeenkaur ashmeenkaur force-pushed the create_new_config_from_legacy branch from d7a150e to 35b0808 Compare June 24, 2024 17:09
@ashmeenkaur ashmeenkaur changed the title [Do not review - dependent on other PRs] Create new config object from legacy flags and config Create new config object from legacy flags and config Jun 24, 2024
@ashmeenkaur ashmeenkaur marked this pull request as ready for review June 24, 2024 17:11
@ashmeenkaur ashmeenkaur requested review from kislaykishore and a team as code owners June 24, 2024 17:11
@ashmeenkaur ashmeenkaur requested review from raj-prince and removed request for a team June 24, 2024 17:11
@ashmeenkaur ashmeenkaur removed the execute-integration-tests Run only integration tests label Jun 24, 2024
@ashmeenkaur ashmeenkaur force-pushed the create_new_config_from_legacy branch from 35b0808 to a9490aa Compare June 24, 2024 17:58
@ashmeenkaur ashmeenkaur force-pushed the dont_use_typecasted_strings_in_mount_config branch from 6a77191 to 5c39d59 Compare June 24, 2024 18:19
@ashmeenkaur ashmeenkaur force-pushed the create_new_config_from_legacy branch 2 times, most recently from fd21df6 to 1007005 Compare June 25, 2024 05:00
cmd/legacy_param_converter_test.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter_test.go Outdated Show resolved Hide resolved
@ashmeenkaur ashmeenkaur force-pushed the dont_use_typecasted_strings_in_mount_config branch from 4beb095 to e0e12da Compare June 25, 2024 07:52
@ashmeenkaur ashmeenkaur force-pushed the create_new_config_from_legacy branch from 1007005 to 163957e Compare June 25, 2024 08:32
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter_test.go Outdated Show resolved Hide resolved
cmd/legacy_param_converter_test.go Outdated Show resolved Hide resolved
Base automatically changed from dont_use_typecasted_strings_in_mount_config to CLI_config_parity_release June 25, 2024 12:59
@ashmeenkaur ashmeenkaur force-pushed the create_new_config_from_legacy branch from 163957e to 9256265 Compare June 25, 2024 13:06
cmd/legacy_param_mapper.go Outdated Show resolved Hide resolved
cmd/legacy_param_mapper.go Outdated Show resolved Hide resolved
cmd/legacy_param_mapper.go Outdated Show resolved Hide resolved
cmd/legacy_param_mapper.go Outdated Show resolved Hide resolved
cmd/legacy_param_mapper_test.go Outdated Show resolved Hide resolved
cmd/legacy_param_mapper_test.go Outdated Show resolved Hide resolved
@ashmeenkaur ashmeenkaur requested review from kislaykishore and removed request for raj-prince June 26, 2024 09:36
}
}

func TestOverrideWithFlag(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We only need to test the API i.e. the public functions. Since we are already testing overlapping flags in the previous test, I think this is not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will remove in follow up PR

@ashmeenkaur ashmeenkaur merged commit 492f2ba into CLI_config_parity_release Jun 26, 2024
13 checks passed
@ashmeenkaur ashmeenkaur deleted the create_new_config_from_legacy branch June 26, 2024 11:39
kislaykishore pushed a commit that referenced this pull request Jun 28, 2024
* changes to populate config

* remove unecessary change

* add client protocol changes

* review comments

* review comment

* rebase changes

* review comments

* review comment + unit tests

* review comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants