-
Notifications
You must be signed in to change notification settings - Fork 672
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
internal/contour: Refactor v2 contour files into v2 package #2928
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2928 +/- ##
=======================================
Coverage 74.91% 74.91%
=======================================
Files 87 87
Lines 5604 5604
=======================================
Hits 4198 4198
Misses 1315 1315
Partials 91 91
|
88087be
to
bd45169
Compare
ready for a rebase |
bd45169
to
f4cecd8
Compare
all rebased @skriss |
Refactors the contour package into a v2 package for Envoy API V2 types. Common items are kept in the contour package for use by other versions. This doesn't change anything functionally, just refactors contour into a v2 package. Updates: projectcontour#1898 Signed-off-by: Steve Sloka <slokas@vmware.com>
f4cecd8
to
8056161
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I do think we could now consider moving internal/contour/v2
into a better-named package (e.g. internal/xdscache/v2
or something) now that they're separated out from the other things in internal/contour
, but I'm fine following up on that later.
@@ -29,6 +29,7 @@ import ( | |||
contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" | |||
"github.com/projectcontour/contour/internal/annotation" | |||
"github.com/projectcontour/contour/internal/contour" | |||
contourv2 "github.com/projectcontour/contour/internal/contour/v2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a little unfortunate that contourv1
refers to the API package, while contourv2
refers to the internal package. We could use contourv1api
or similar for the former. Not blocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup I agree. =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened #2938.
I also opened #2939 for the other naming-related comment. |
Refactors the contour package into a v2 package for Envoy API V2 types.
Common items are kept in the contour package for use by other versions.
This doesn't change anything functionally, just refactors contour into a v2 package.
Updates: #1898
Blocked waiting on #2921 to merge.