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

internal: merge xds/envconfig into env package #4923

Merged
merged 2 commits into from
Nov 11, 2021
Merged

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Nov 2, 2021

Also contains #4922

RELEASE NOTES: N/A

@dfawley dfawley added the Type: Internal Cleanup Refactors, etc label Nov 2, 2021
@dfawley dfawley added this to the 1.43 release milestone Nov 2, 2021
@zasweq
Copy link
Contributor

zasweq commented Nov 3, 2021

LGTM, but see my comment on #4922.

@dfawley
Copy link
Member Author

dfawley commented Nov 9, 2021

PR updated with the changes made in #4922 to leave RBAC in place.

@@ -79,11 +79,11 @@ func SetupBootstrapFile(opts BootstrapOptions) (func(), error) {
}
logger.Infof("Created bootstrap file at %q with contents: %s\n", f.Name(), bootstrapContents)

origBootstrapFileName := env.BootstrapFileName
env.BootstrapFileName = f.Name()
origBootstrapFileName := envconfig.BootstrapFileName
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, most (all?) of the xds variable names don't have "xds" in it. Would that be confusing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I tweaked the variable names a bit; PTAL

@dfawley dfawley assigned dfawley and unassigned menghanl and zasweq Nov 11, 2021
@dfawley dfawley merged commit d61c7ae into grpc:master Nov 11, 2021
@dfawley dfawley deleted the envmerge branch November 11, 2021 01:05
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants