-
Notifications
You must be signed in to change notification settings - Fork 439
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
contrib/labstack/echo.v4: added support for Echo v4 #548
Conversation
Copied the contents of contrib/labstack/echo to echo.v4 and appended /v4 to any imports that referenced echo. Fixes: DataDog#543
There might be a mistake somewhere, we're getting:
|
That error seems to be because it's not set up as a module (with Adding the module details (with |
@cgilmour you beat me to it. I was going to see about adding modules but didn't quite get to it before you. As you pointed out this is likely to have some larger side effects to how this is maintained going forward. Also I wasn't sure if we could get away with adding modules within |
We did some experimenting in the past, and the conclusion was that modules aren't easy to support with the current repo layout and import path. There's a few solutions we could go with, and they'll get judged on their impact on end users as well as maintainers/contributors. |
Oh my! Indeed this means we've hit our first problem caused by us not supporting modules. While support for multi-module repos is possible today (golang/go#27056), I'm frankly still not fully comfortable that this is a good move and it seems like it might bring a lot of trouble and difficulties (golang/go#26640, golang/go#27542, golang/go#28835). If the pressure becomes high, and more problems like this come up, we'll be forced to switch to modules, but I'd like to prolong that decision as much as possible, hoping that the ecosystem will present solutions to the issues mentioned above. I wondering if it would work to make a module only of this integration, and leave the rest as is, or if we'd have to go all in? Relates to #471 |
Any update on this? |
No updates yet. Please follow the related issue #471. We will post updates there as soon as we start working on this... |
#698 has been merged and targeted for 1.26.0 release |
Copied the contents of contrib/labstack/echo to echo.v4 and appended /v4
to any imports that referenced echo.
Fixes: #543