Skip to content
This repository was archived by the owner on Mar 6, 2025. It is now read-only.

Conversation

@hecomp
Copy link

@hecomp hecomp commented Jun 22, 2023

  • Added template validation.

@hecomp hecomp changed the title Hsilva/validation template hsilva/validation template Jun 22, 2023
@@ -0,0 +1,3 @@
module github.com/lightstep/integrations

go 1.19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
go 1.19
go 1.20

package text

import (
"io/ioutil"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: contents of "ioutil" have migrated to either "os" or "io". In general, preferred to use the versions in those packages as "ioutil" is deprecated.

@@ -0,0 +1,147 @@
package text
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For unit tests, using package text_test provides some useful guarantees. It requires writing tests in the same way you'll use the code.

Copy link
Contributor

@nslaughter nslaughter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made 3 comments: (1) is the go.mod version, (2) is to switch to test the package using the package {{pkg name}}_test form, (3) is switch from ioutil to new versions of the functions - including new names for temp fs related things (see comments).

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Create a temp file
tempFile, err := ioutil.TempFile("", "template")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually when I'm looking for the old ioutil functions I expect to find the function lifted and shifted to io or os. But for the temp fs functions they made new function names - os.CreateTemp for new files and os.MkdirTemp for directories.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants