Skip to content

feat: support user-facing version#609

Merged
Xunzhuo merged 2 commits into
envoyproxy:mainfrom
Xunzhuo:feat-version
Nov 3, 2022
Merged

feat: support user-facing version#609
Xunzhuo merged 2 commits into
envoyproxy:mainfrom
Xunzhuo:feat-version

Conversation

@Xunzhuo
Copy link
Copy Markdown
Member

@Xunzhuo Xunzhuo commented Oct 19, 2022

Resolves: #498

Fixes: #615

Requires: #639

image

Signed-off-by: bitliu bitliu@tencent.com

@Xunzhuo Xunzhuo requested a review from a team as a code owner October 19, 2022 04:58
@Xunzhuo Xunzhuo force-pushed the feat-version branch 2 times, most recently from 7f32616 to c72cd68 Compare October 19, 2022 05:04
@arkodg arkodg requested a review from LukeShu October 19, 2022 05:05
@Xunzhuo Xunzhuo force-pushed the feat-version branch 2 times, most recently from 01a8f23 to 7b1e508 Compare October 19, 2022 06:41
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 19, 2022

Codecov Report

Merging #609 (adf2c01) into main (10eb279) will increase coverage by 0.15%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##             main     #609      +/-   ##
==========================================
+ Coverage   63.54%   63.69%   +0.15%     
==========================================
  Files          47       47              
  Lines        5766     5749      -17     
==========================================
- Hits         3664     3662       -2     
+ Misses       1877     1862      -15     
  Partials      225      225              
Impacted Files Coverage Δ
internal/cmd/versions.go 50.00% <0.00%> (+21.42%) ⬆️
internal/cmd/root.go 100.00% <100.00%> (ø)
internal/provider/kubernetes/httproute.go 59.25% <0.00%> (-8.24%) ⬇️
internal/provider/kubernetes/gatewayclass.go 71.73% <0.00%> (+1.44%) ⬆️
internal/provider/kubernetes/gateway.go 51.59% <0.00%> (+1.91%) ⬆️
internal/provider/kubernetes/tlsroute.go 62.11% <0.00%> (+2.64%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Xunzhuo Xunzhuo force-pushed the feat-version branch 5 times, most recently from 4cb7b11 to 89ccea5 Compare October 19, 2022 14:21
Copy link
Copy Markdown
Contributor

@danehans danehans left a comment

Choose a reason for hiding this comment

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

Looks like you have stale quickstart-related changes that should be removed from this PR.

Comment thread docs/conf.py Outdated
Comment thread README.md Outdated
@danehans
Copy link
Copy Markdown
Contributor

danehans commented Oct 19, 2022

IMHO having too many options can cause confusion. I prefer envoy-gateway version and the output is the EG, Gateway API, Envoy versions, and the commit SHA.

Thoughts @arkodg @youngnick @skriss @LukeShu @AliceProxy

@Alice-Lilith
Copy link
Copy Markdown
Member

Plus one on keeping it simple. For example, you can go version but not also go --version so there is an established pattern for only supporting one style.

@Xunzhuo
Copy link
Copy Markdown
Member Author

Xunzhuo commented Oct 19, 2022

Yes I am intending to just keep eg version.

@danehans danehans added the documentation Improvements or additions to documentation label Oct 19, 2022
@danehans danehans added this to the 0.2.0 milestone Oct 19, 2022
@Xunzhuo
Copy link
Copy Markdown
Member Author

Xunzhuo commented Oct 19, 2022

I have removed the --version for syncing the style.

But need some feedback from #615 (comment).

The eg version will be updated normally well and automatically in local, but wil not be in the GA, missing the Git Tree infos.

That is why tools/goversion can not work in GA as well, and causing #615.

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Oct 19, 2022

IMHO having too many options can cause confusion. I prefer envoy-gateway version and the output is the EG, Gateway API, Envoy versions, and the commit SHA.

Thoughts @arkodg @youngnick @skriss @LukeShu @AliceProxy

  • 1 on a single pattern, either envoy-gateway version or envoy-gateway --version
    dont have a strong opinion on either

Copy link
Copy Markdown
Member Author

@Xunzhuo Xunzhuo left a comment

Choose a reason for hiding this comment

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

/wait

I am not sure and not recomment to put this into v0.2.0, I think a couple of things need more discussion.

@danehans danehans modified the milestones: 0.2.0, 0.3.0-rc.1 Oct 19, 2022
@danehans danehans marked this pull request as draft October 19, 2022 20:43
@danehans
Copy link
Copy Markdown
Contributor

/wait
I am not sure and not recomment to put this into v0.2.0, I think a couple of things need more discussion.

@Xunzhuo converted to draft in the meantime.

Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo marked this pull request as ready for review November 3, 2022 03:03
@Xunzhuo Xunzhuo merged commit bb73c68 into envoyproxy:main Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing release version in docs site Make envoy-gateway --version properly user-facing

5 participants