Switch from x/net/context -> context#2305
Conversation
|
We tried this before but because GRPC still uses the old context, the service implementations will not compile because they don't fulfill the interface. |
|
Didn't they update |
|
For go1.9+ x/net/context it's just an alias to to the stdlib context. |
|
Sorry I missed it, needed to re-vendor x/net, commit added. |
|
rebased; changed the order of commits to not break git bisect |
|
Do we need to cherry-pick this to v1.1.1 for using containerd 1.1 in Moby? |
This version includes "x/net/context" which is fully compatible with the standard Go "context" package, so the two can be mixed together. Signed-off-by: Kir Kolyshkin <[email protected]>
Since Go 1.7, "context" is a standard package, superceding the "x/net/context". Since Go 1.9, the latter only provides type aliases from the former. Therefore, it makes sense to switch to the standard package, and the change is not disruptive in any sense. This commit deals with a few cases where both packages happened to be imported by the same source file. A choice between "context" and "gocontext" was made for each file in order to minimize the patch. Signed-off-by: Kir Kolyshkin <[email protected]>
Since Go 1.7, context is a standard package, superceding the
"x/net/context". Since Go 1.9, the latter only provides a few type
aliases from the former. Therefore, it makes sense to switch to the
standard package.
This commit was generated by the following script (with a couple of
minor fixups to remove extra changes done by goimports):
#!/bin/bash
if [ $# -ge 1 ]; then
FILES=$*
else
FILES=$(git ls-files \*.go | grep -vF ".pb.go" | grep -v
^vendor/)
fi
for f in $FILES; do
printf .
sed -i -e 's|"golang.org/x/net/context"$|"context"|' $f
goimports -w $f
awk ' /^$/ {e=1; next;}
/[[:space:]]"context"$/ {e=0;}
{if (e) {print ""; e=0}; print;}' < $f > $f.new && \
mv $f.new $f
goimports -w $f
done
echo
Signed-off-by: Kir Kolyshkin <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2305 +/- ##
=======================================
Coverage 45.54% 45.54%
=======================================
Files 83 83
Lines 9185 9185
=======================================
Hits 4183 4183
Misses 4326 4326
Partials 676 676
Continue to review full report at Codecov.
|
|
LGTM |
|
LGTM
What would be the concern here? My understanding is there is no functional difference and it is already compiling with Moby no problem. |
👍 |
Since Go 1.7, "context" is a standard package, superceding the "x/net/context". Since Go 1.9, the latter only provides type aliases from the former. Therefore, it makes sense to switch to the standard package, and the change is not disruptive in any sense.
The first commit deals with a few cases where both packages happened to be imported by the same source file. A choice between "context" and "gocontext" was made for each file in order to minimize the patch.
The second commit is generated by a shell script, and deals with the rest of the code.
Similar changes to moby/moby: moby/moby#36904.