Skip to content
This repository was archived by the owner on Oct 12, 2021. It is now read-only.

Repository renamed to "Routing"#7

Merged
tzununbekov merged 1 commit into
mainfrom
routing-rename
Apr 22, 2021
Merged

Repository renamed to "Routing"#7
tzununbekov merged 1 commit into
mainfrom
routing-rename

Conversation

@tzununbekov
Copy link
Copy Markdown
Member

@tzununbekov tzununbekov commented Apr 21, 2021

Filter repository renamed to Routing
Related to #5

@tzununbekov tzununbekov marked this pull request as ready for review April 21, 2021 13:21
Comment thread cmd/filter-deployment/main.go Outdated
PodName string `envconfig:"POD_NAME" required:"true"`
ContainerName string `envconfig:"CONTAINER_NAME" required:"true"`
Port int `envconfig:"FILTER_PORT" default:"8080"`
Port int `envconfig:"FILTER_PORT" default:"8080"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a question since this code already existed before this PR: what's the case for making this configurable?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

tbh, I cannot make up the real case when this may need to be changed, but at the same time cannot see anything harmful in it either. I'd say that we can leave it until the next time we change something in that part of the code.

Comment thread config/filter.yaml
Comment thread go.mod Outdated
@tzununbekov
Copy link
Copy Markdown
Member Author

ping dear reviewers! let's finish this up!

Copy link
Copy Markdown
Member

@sebgoa sebgoa left a comment

Choose a reason for hiding this comment

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

looks good from a brief check

@tzununbekov tzununbekov merged commit c819966 into main Apr 22, 2021
@tzununbekov tzununbekov deleted the routing-rename branch April 22, 2021 08:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants