Skip to content

Write Go files with an internal helper instead of gofiles.Write#222

Open
tabboud wants to merge 1 commit into
masterfrom
ta/write-go-files
Open

Write Go files with an internal helper instead of gofiles.Write#222
tabboud wants to merge 1 commit into
masterfrom
ta/write-go-files

Conversation

@tabboud

@tabboud tabboud commented Oct 14, 2022

Copy link
Copy Markdown

Before this PR

A break (no more template rendering) and a behavior change (using packages.Load to get module paths instead of $GOPATH) in nmiyake/pkg/gofiles caused tests to fail on palantir/godel-okgo-asset-importalias#132 since we're no longer writing go.mod files.

All usages of gofiles.Write did not inspect the returned import paths, so instead of using that function, we can use an internal function to write out the raw Go files avoiding any package loading and only returning errors for writing the files.

This is almost identical to what was done for distgo here: palantir/distgo#237

After this PR

==COMMIT_MSG==
Write Go files with an internal helper instead of gofiles.Write
==COMMIT_MSG==

Possible downsides?

@nmiyake nmiyake left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on the fact that this is only file that changes, it seems like this change alone doesn't remove the dependency for gofiles -- is that correct? It feels like this change would make more sense if it removed the gofiles dependency from this module (if not fully/transitively, then at least for direct reference)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants