-
Notifications
You must be signed in to change notification settings - Fork 373
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
fix(cmd/gno/clean): allow to run gno clean -modcache
from anywhere + rename and use gnomod.ModCachePath
#3083
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Norman Meier <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3083 +/- ##
==========================================
+ Coverage 63.34% 63.35% +0.01%
==========================================
Files 548 548
Lines 78680 78681 +1
==========================================
+ Hits 49836 49848 +12
+ Misses 25482 25468 -14
- Partials 3362 3365 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Norman Meier <[email protected]>
951f6fa
to
76fa49c
Compare
Signed-off-by: Norman Meier <[email protected]>
gnovm/cmd/gno/main_test.go
Outdated
if test.tmpGnoHome { | ||
gnohome, err := os.MkdirTemp(os.TempDir(), "gnotesthome_") | ||
require.NoError(t, err) | ||
t.Cleanup(func() { os.RemoveAll(gnohome) }) | ||
t.Setenv("GNOHOME", gnohome) | ||
} |
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 can just be done always, I would say, rather than with the flag
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.
okay but this breaks some other tests, I'd rather do it separately from this
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.
actually it was straightforward, I reverted the condition and disabled the tmp gnohome on the 3 offending test
gnovm/cmd/gno/clean.go
Outdated
@@ -64,6 +63,19 @@ func execClean(cfg *cleanCfg, args []string, io commands.IO) error { | |||
return flag.ErrHelp | |||
} | |||
|
|||
if cfg.modCache { | |||
modCacheDir := gnomod.GetGnoModPath() |
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.
Can we rename the method to ModCachePath
?
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.
if cfg.dryRun || cfg.verbose { | ||
io.Println("rm -rf", modCacheDir) | ||
} | ||
return nil |
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.
Remove? (ie. support cleaning modcache as well as cached files in current dir?)
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.
Maybe we can return here (without an error, maybe a log message) if there is no gno.mod.
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.
are you sure we want that, I follow go behavior here:
the dry run with no arguments shows only a removal of the modcache
❯ go clean -modcache -n
rm -rf /Users/norman/go/pkg/mod
with arguments it's explicitely disabled
❯ go clean -modcache .
go: clean -modcache cannot be used with package arguments
Signed-off-by: Norman Meier <[email protected]>
gno clean -modcache
from anywhere + use GetGnoModPath
gno clean -modcache
from anywhere + rename and use gnomod.ModCachePath
Signed-off-by: Norman Meier <[email protected]>
Signed-off-by: Norman Meier <[email protected]>
gno clean
, run the-modcache
case before checking for presence of agno.mod
to allow to rungno clean -modcache
from anywhere (like go)gno clean -modcache
to use thegnomod.GetGnoModPath
helper to get the modcache pathgnomod.GetGnoModPath
->gnomod.ModCachePath
gno
cmd tests by using a tmpGNOHOME
instead of the system oneContributors' checklist...
BREAKING CHANGE: xxx
message was included in the description