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

source: Reorder AllowSource switch Statement and Comment Nits #15696

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

nathanjsweet
Copy link
Member

The AllowSource method contains logic that maps the
hierarchical sources of truth for when an entity's existing
source allows a new source of truth to overwrite it.

The method was written so that the cases were not in the order
of their importance, which makes the method confusing to read.
Additionally there were some typos and incorrectly worded comments
that further make the method difficult to read.

Signed-off-by: Nate Sweet nathanjsweet@pm.me

The AllowSource method contains logic that maps the
hierarchical sources of truth for when an entity's existing
source allows a new source of truth to overwrite it.

The method was written so that the cases were not in the order
of their importance, which makes the method confusing to read.
Additionally there were some typos and incorrectly worded comments
that further make the method difficult to read.

Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
@nathanjsweet nathanjsweet added area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/misc This PR makes changes that have no direct user impact. labels Apr 14, 2021
@nathanjsweet nathanjsweet requested a review from a team as a code owner April 14, 2021 17:13
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Apr 14, 2021
@qmonnet
Copy link
Member

qmonnet commented Apr 14, 2021

test-me-please

@qmonnet qmonnet added the kind/cleanup This includes no functional changes. label Apr 14, 2021
@qmonnet
Copy link
Member

qmonnet commented Apr 14, 2021

test-runtime
(failed to provision VM)

@pchaigno
Copy link
Member

pchaigno commented Apr 20, 2021

The previous occurrence failed because of a short build breakage on master, fixed since then.

test-runtime

@pchaigno
Copy link
Member

pchaigno commented Apr 20, 2021

A couple tests failed in 1.21-4.9. Let's see if they are flakes. If they are, we need to open issues.

test-1.21-4.9

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 20, 2021
@christarazi christarazi merged commit 613d02c into master Apr 20, 2021
1.10.0 automation moved this from In progress to Done Apr 20, 2021
@christarazi christarazi deleted the pr/nathanjsweet/comment-case-nits-for-source branch April 20, 2021 19:11
@pchaigno
Copy link
Member

A couple tests failed in 1.21-4.9. Let's see if they are flakes. If they are, we need to open issues.

I opened #15791 for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants