Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only generate moq when interface file is newer than output file #179

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

djui
Copy link
Contributor

@djui djui commented Oct 12, 2022

No description provided.

Copy link
Contributor

@breml breml left a comment

Choose a reason for hiding this comment

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

There are some more cases to be considered:

  1. It is possible to generate mocks from type aliases. In this case, the file where the type alias is located, does not change, when the signature of the mock it self changes.
  2. If the import path of the package, where the interface which is mocked is located, is changed (e.g. by moving a package to a an other folder), the generated mock would be needed to be regenerated, but the timestamp of the source file is not necessarily altered.
  3. The go:generate line is not necessarily in the same file as the interface that is mocked, but changing this line would also potentially change the result of the generated mock.
  4. Whenever moq is updated, the resulting mock file might change as well.

@@ -1,4 +1,4 @@
module github.com/matryer/moq
module github.com/djui/moq
Copy link
Contributor

Choose a reason for hiding this comment

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

This change should not be part of the PR.

(see also all other files, where the import paths are changed)

@@ -37,6 +38,7 @@ func main() {
flag.BoolVar(&flags.skipEnsure, "skip-ensure", false,
"suppress mock implementation check, avoid import cycle if mocks generated outside of the tested package")
flag.BoolVar(&flags.remove, "rm", false, "first remove output file, if it exists")
flag.BoolVar(&flags.force, "force", false, "force generation, otherwise check if go generate file is newer than output file")
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is not backwards compatible. Maybe it is better to preserve the current mode of operation and add a flag to opt-in for the mtime behavior. Maybe the -mtime flag could be used to enable the mtime mode as well as contain the name of the source file as well. If the source file is the empty string, it would fall back to GOFILE env variable.

main.go Outdated
Comment on lines 72 to 77
if inStat, err := os.Stat(inFile); err != nil {
fmt.Fprintln(os.Stderr, err)
} else if outStat, err := os.Stat(flags.outFile); err != nil {
if !errors.Is(err, os.ErrNotExist) {
fmt.Fprintln(os.Stderr, err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If the mtime flag is used instead of force, I think failing to stat the source or the destination file would be an error, which is returned from run.

@matryer
Copy link
Owner

matryer commented Nov 12, 2022

I like this, if it's opt-in @djui - the build is failing, but if you can patch it up, we can merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants