Skip to content

chore: Fix column name in proxy ls command #7450

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

Merged
merged 1 commit into from
May 8, 2023

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented May 8, 2023

NAME              URL                           PROXY STATUS  
brazil-saopaulo   https://brazil.dev.coder.com  ok            
europe-frankfurt  https://europe.dev.coder.com  ok            
sydney            https://sydney.dev.coder.com  ok 

@Emyrk Emyrk requested a review from bpmct May 8, 2023 15:42
Copy link
Member

@bpmct bpmct left a comment

Choose a reason for hiding this comment

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

Does it have to be called proxy status? To me, Status is more natural. It feels inconsistent. Wouldn't all be proxy name, proxy url, proxy status?

@Emyrk
Copy link
Member Author

Emyrk commented May 8, 2023

@bpmct I lokoed into removing the word Proxy to make it just Status, but it requires a lot more code changes.

It gets a bit tricky because the recursive nesting naming is how we uniquely identify some struct field. So I need to add another tag to indicate ignore the parent name, or pass down some options...

tl;dr it gets more tricky. And maybe it is worth it, but for now this makes things better and I can spend the time to make this Status when there is less pressing things imo. I'll have spare time next week to take on these more minor tasks.

@Emyrk Emyrk merged commit 1aac820 into main May 8, 2023
@Emyrk Emyrk deleted the stevenmasley/proxy_column_fix branch May 8, 2023 17:23
@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2023
@bpmct
Copy link
Member

bpmct commented May 15, 2023

Ah, sorry I didn't respond. Makes sense. There are bigger fish to fry

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.

2 participants