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

clarify logic in endpointstranslator.go #5800

Closed
skriss opened this issue Oct 3, 2023 · 2 comments · Fixed by #5927
Closed

clarify logic in endpointstranslator.go #5800

skriss opened this issue Oct 3, 2023 · 2 comments · Fixed by #5927
Assignees
@xiangpingjiang
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Milestone
1.28.0

Comments

@skriss
Copy link
Member

skriss commented Oct 3, 2023

ref. #5745 (comment)
ref.

contour/internal/xdscache/v3/endpointstranslator.go

Line 57 in 7feb49e

if (healthPort.Protocol != p.Protocol || port.Protocol != p.Protocol) && p.Protocol != v1.ProtocolTCP {

ref. #5745 (comment)

This line, and the (soon-to-be-merged) same line in endpointslicetranslator.go, could be rewritten for clarity. I'd suggest possibly breaking it into multiple independent short-circuits.
@skriss skriss added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Oct 3, 2023
@skriss skriss added this to Contour Oct 4, 2023
@skriss skriss moved this to Todo in Contour Oct 4, 2023
@skriss skriss added this to the 1.28.0 milestone Oct 4, 2023
@xiangpingjiang
Copy link
Contributor

@skriss
hello, Kriss, I would like to work on this issue. Can you please assign me?

@skriss
Copy link
Member Author

skriss commented Oct 5, 2023

Sure, I would suggest waiting until #5745 is merged though, so it can be addressed in both places at the same time.

@xiangpingjiang xiangpingjiang mentioned this issue Nov 1, 2023
Merged
@github-project-automation github-project-automation bot moved this from Todo to Done in Contour Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xiangpingjiang xiangpingjiang

Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
Contour
Status: Done
Milestone
1.28.0
Development

Successfully merging a pull request may close this issue.

clarify logic in endpointstranslator.go xiangpingjiang/contour
2 participants
@skriss @xiangpingjiang