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

refactor: remove BaseURL and AdminKey in config #826

Merged
merged 1 commit into from
Jan 5, 2022
Merged

refactor: remove BaseURL and AdminKey in config #826

merged 1 commit into from
Jan 5, 2022

Conversation

zaunist
Copy link
Contributor

@zaunist zaunist commented Jan 2, 2022

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What this PR does / why we need it:

Remove BaseURL and AdminKey.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2022

Codecov Report

Merging #826 (0c7be4f) into master (990971d) will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #826      +/-   ##
==========================================
- Coverage   32.14%   32.09%   -0.06%     
==========================================
  Files          71       71              
  Lines        7727     7721       -6     
==========================================
- Hits         2484     2478       -6     
  Misses       4968     4968              
  Partials      275      275              
Impacted Files Coverage Δ
cmd/ingress/ingress.go 77.55% <ø> (-0.45%) ⬇️
pkg/config/config.go 64.64% <ø> (-1.38%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae69cd3...0c7be4f. Read the comment docs.

@zaunist zaunist marked this pull request as ready for review January 2, 2022 17:15
Copy link
Contributor

@tokers tokers left a comment

Choose a reason for hiding this comment

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

This is a broken change and we should add some hints for users.

@tao12345666333 tao12345666333 linked an issue Jan 3, 2022 that may be closed by this pull request
@tao12345666333 tao12345666333 added this to In progress in v1.5 Planning via automation Jan 3, 2022
@tao12345666333 tao12345666333 added area/controller changelog Issues with this label should be added to changelog when public a new release labels Jan 3, 2022
v1.5 Planning automation moved this from In progress to Reviewer approved Jan 3, 2022
Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

LGTM.

@tao12345666333
Copy link
Member

Thanks! The code LGTM.

I added the do-not-merge/hold label, and I want to perform additional checks and remind more people to pay attention to this change.

Will be merged after the bi-weekly meeting next week.

@tao12345666333 tao12345666333 self-requested a review January 3, 2022 05:11
@tao12345666333 tao12345666333 self-assigned this Jan 3, 2022
@zaunist
Copy link
Contributor Author

zaunist commented Jan 3, 2022

This is a broken change and we should add some hints for users.

Sure, I think so.

Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

LGTM

@tao12345666333 tao12345666333 merged commit e40cc31 into apache:master Jan 5, 2022
v1.5 Planning automation moved this from Reviewer approved to Done Jan 5, 2022
@zaunist zaunist deleted the feat/remove_adminkey branch January 5, 2022 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller changelog Issues with this label should be added to changelog when public a new release do-not-merge/hold
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

clean up: remove BaseURL and AdminKey in config
5 participants