- Notifications
You must be signed in to change notification settings - Fork928
fix: include subdirectories in example templates#1715
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
examples/examples.go Outdated
| _, err = tarWriter.Write(data) | ||
| if err != nil { | ||
| return xerrors.Errorf("write: %w", err) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You should be able to io.Copy instead to avoid the extra buffer
examples/examples.go Outdated
| if err != nil { | ||
| return xerrors.Errorf("open file %s: %w", path, err) | ||
| } | ||
| _, err = file.Read(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This doesn't matter if you use io.Copy, but in the future you should use io.ReadAll instead, because Read isn't guaranteed to fill your output buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good catch!
| @@ -154,22 +155,17 @@ func Archive(exampleID string) ([]byte, error) { | |||
| } else { | |||
| header.Name = path | |||
| data := make([]byte, info.Size()) | |||
| file, err := exampleFiles.Open(path) | |||
| if err != nil { | |||
| return xerrors.Errorf("open file %s: %w", path, err) | |||
| } | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should be closed i.e.defer file.Close()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good point. Technically I think we would be OK without it, because the readers returned byembed.FS are just pointers, but I added it anyway to make the code more obviously correct.
This PR makes
coder templates initcorrectly include files in subdirectories of examples.Subtasks
examples/templatesand updated package to embed that entire directoryexamples.Archiveto recursively walk directoriesFixes#1669