Modernize Go compatibility: modules, deprecated APIs, unit tests, CI matrix#124
Conversation
… update, unit tests Co-authored-by: wendal <589819+wendal@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Modernizes the project’s Go toolchain and developer workflow by introducing Go modules, updating deprecated standard-library APIs, and adding CI + unit tests to validate core behavior.
Changes:
- Added Go module support (
go.mod/go.sum) and updated GitHub Actions workflow to build/test/vet across multiple Go versions. - Replaced deprecated
io/ioutilusage andos.SEEK_SETwith their modern equivalents. - Added/updated tests and fixed a
go vet-flagged formatting bug inmap2struct.go.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Introduces module definition and Go language/toolchain version requirement. |
go.sum |
Records module dependency checksums for reproducible builds. |
.github/workflows/go.yml |
Updates CI to a Go version matrix and adds go test/go vet. |
config.go |
Migrates from ioutil.ReadAll to io.ReadAll. |
payload.go |
Migrates ioutil.ReadAll to io.ReadAll and os.SEEK_SET to io.SeekStart. |
compile.go |
Migrates ioutil.WriteFile/ioutil.ReadDir to os.WriteFile/os.ReadDir. |
widgets.go |
Migrates ioutil.ReadAll to io.ReadAll. |
post.go |
Migrates ioutil.WriteFile/ioutil.ReadDir to os.WriteFile/os.ReadDir. |
gor/new.go |
Migrates ioutil.ReadAll to io.ReadAll. |
gor/gor.go |
Migrates ioutil.ReadFile to os.ReadFile. |
map2struct.go |
Fixes incorrect fmt.Sprint usage to fmt.Sprintf. |
payload_test.go |
Skips TestPayLoad when the hardcoded test directory is missing. |
gor_test.go |
Adds unit tests for URL/path encoding, markdown rendering, sorting, and Mapper helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| os.MkdirAll(filepath.Dir(dstPath), os.ModePerm) | ||
| if HTML_EXT != "" { | ||
| content += HTML_EXT | ||
| } | ||
| ioutil.WriteFile(dstPath, []byte(content), os.ModePerm) | ||
| os.WriteFile(dstPath, []byte(content), os.ModePerm) | ||
| } |
There was a problem hiding this comment.
os.WriteFile returns an error that is currently being ignored. If writing the compiled output fails (permissions, disk full, invalid path), the build can silently produce incomplete output. Capture and handle the error (return it to the caller or log/fail fast).
There was a problem hiding this comment.
Fixed in 6cab6d8 — the error from os.WriteFile is now captured and logged via log.Println.
| return err | ||
| } | ||
| f.Seek(0, os.SEEK_SET) | ||
| f.Seek(0, io.SeekStart) |
There was a problem hiding this comment.
The result of f.Seek(0, io.SeekStart) is ignoring the returned error. If the seek fails, the subsequent layout parsing will read from an unexpected offset and can mis-parse templates. Handle the error from Seek (and ideally the returned offset) and propagate/return it from the filepath.Walk callback.
| f.Seek(0, io.SeekStart) | |
| if _, err = f.Seek(0, io.SeekStart); err != nil { | |
| return err | |
| } |
There was a problem hiding this comment.
Fixed in 6cab6d8 — applied your suggestion exactly; the error from f.Seek is now checked and returned to the filepath.Walk callback.
|
|
||
| decoder := base64.NewDecoder(base64.StdEncoding, bytes.NewBufferString(INIT_ZIP)) | ||
| b, _ := ioutil.ReadAll(decoder) | ||
| b, _ := io.ReadAll(decoder) |
There was a problem hiding this comment.
io.ReadAll(decoder) errors are being discarded. If the embedded base64 content is malformed or the decoder fails, this can proceed with a partial/empty buffer and then fail later with less actionable errors. Handle the read error and abort the command (e.g., log.Fatal) with a clear message.
| b, _ := io.ReadAll(decoder) | |
| b, err := io.ReadAll(decoder) | |
| if err != nil { | |
| log.Fatalf("failed to read embedded init zip data: %v", err) | |
| } |
There was a problem hiding this comment.
Fixed in 6cab6d8 — applied your suggestion; the error from io.ReadAll is now checked and causes log.Fatalf with a clear message.
| d, _ := os.ReadFile("gor-content.zip") | ||
| _zip, _ := os.OpenFile("zip.go", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, os.ModePerm) | ||
| header := `package main |
There was a problem hiding this comment.
This branch ignores errors from os.ReadFile/os.OpenFile, and then writes to the returned file handles. If either call fails (missing zip, permissions), it can panic or generate a corrupt zip.go. Handle these errors and abort the command with a clear log.Fatal message.
There was a problem hiding this comment.
Fixed in 6cab6d8 — both os.ReadFile and os.OpenFile errors are now checked and abort with log.Fatal and descriptive messages.
|
@copilot apply changes based on the comments in this thread |
…enFile Co-authored-by: wendal <589819+wendal@users.noreply.github.com>
go.mod) for modern Go compatibilityio/ioutilusages →io/osequivalents (Go 1.16+)os.SEEK_SET→io.SeekStart.github/workflows/go.yml) to use modern Go and actionsfmt.Sprint("%v", v)bug inmap2struct.goTestPayLoadto skip gracefully when test data is unavailableos.WriteFileincompile.go(log on failure)f.Seekinpayload.go(return error to Walk callback)io.ReadAllingor/new.go(log.Fatal with clear message)os.ReadFile/os.OpenFileingor/gor.go(log.Fatal with clear messages)💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.