add CopyCoreVersion #223
add CopyCoreVersion #223
Conversation
xhd2015
force-pushed
the
add-copy-core-version
branch
from
58d5046 to
e64e4df
Compare
| @@ -15,7 +15,7 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ ubuntu-latest] | |||
| go: [ '1.18' ,'1.20','1.21'] | |||
| go: [ '1.17','1.18','1.19','1.20','1.21'] | |||
There was a problem hiding this comment.
You know 1.22 is the last one right? 😁
There was a problem hiding this comment.
That's exactly why it is not here, because this file only tests older go versions. Cutting edge go is tested in another file.
Glad you check the PR so carefully. Thanks a lot. 😊
There was a problem hiding this comment.
Maybe a comment just above the line became it's not obvious when seeing the line changed in a PR
|
|
||
| func CopyCoreVersion(xgoVersionFile string, runtimeVersionFile string) error { | ||
| // copy xgo's core version to runtime version | ||
| code, err := ioutil.ReadFile(xgoVersionFile) |
There was a problem hiding this comment.
You kept ioutil for compatibility purpose?
https://pkg.go.dev/io/ioutil#ReadFile
I don't think you should care about go before 1.16
But if you need it, then please add a comment here about why you are using this deprecated function
There was a problem hiding this comment.
Yes I kept using ioutil for compatibility reason. I expect go1.16 to be the smallest version xgo can support
There was a problem hiding this comment.
Then it recommend a comment
There was a problem hiding this comment.
But wait ioutil was deprecated with 1.16, so?
There was a problem hiding this comment.
So I'll lower the smallest version to go1.14 😼
There was a problem hiding this comment.
Then a comment aside the import and one where you call ioutil.ReadFile would be appreciated
There was a problem hiding this comment.
That's just a joke, the project's runtime/go.mod really says it is targeting go 1.14, so kept here.
Maybe after a year or two we may finally remove this thing.
No description provided.