feat: addpkg command respects gno.mod file #922
Conversation
github-actions
bot
added
📦 🌐 tendermint v2
harry-hov
force-pushed
the
hariom/addpkg-respect-gnomod
branch
from
43104e0
to
1dd9130
Compare
|
@harry-hov do you think the same behavior should be applied for |
Yeah. I planned to do this in #682. But now I will open a separate PR for that. That will act as a preparatory PR for both #896 and #682. (maybe by tomorrow) |
tm2/pkg/crypto/keys/client/addpkg.go
Outdated
| if cfg.pkgPath == "" { | ||
| return errors.New("pkgpath not specified") | ||
| } |
There was a problem hiding this comment.
we can make the migration smoother by just adding a warning here with a comment to remove later.
There was a problem hiding this comment.
sorry. i didn't get it. what kind of warning?
I think we should also maintain a CHANGELOG and LongHelp?
github-actions
bot
removed
the
📦 ⛰️ gno.land
tm2/pkg/crypto/keys/client/addpkg.go
Outdated
|
|
||
| gno "github.com/gnolang/gno/gnovm/pkg/gnolang" | ||
| "github.com/gnolang/gno/gnovm/pkg/gnomod" |
There was a problem hiding this comment.
This one is forbidden.
Maybe we should wait for TM2 split PR started by @piux2, then you'll have the AddPkg not in tm2 anymore, but in gno.land.
@harry-hov, what about trying to help @piux2 finishing his PR first?
There was a problem hiding this comment.
tm2 already imports gnovm[github.com/gnolang/gno/gnovm](see the line above).
maybe we can let this one slide and will be fixed with TM2 split?
There was a problem hiding this comment.
the split is finalised so the problematic import can be fixed, please merge master :)
harry-hov
force-pushed
the
hariom/addpkg-respect-gnomod
branch
from
1b19dd8
to
c3acd88
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #922 +/- ##
=======================================
Coverage 48.81% 48.81%
=======================================
Files 579 579
Lines 78045 78041 -4
=======================================
- Hits 38099 38098 -1
+ Misses 36860 36857 -3
Partials 3086 3086
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
github-actions
bot
added
the
📦 ⛰️ gno.land
harry-hov
force-pushed
the
hariom/addpkg-respect-gnomod
branch
from
697ef52
to
23ccecf
Compare
There was a problem hiding this comment.
Catching up on old items in the queue. Sorry for the delay.
The changes LGTM though I personally think it would be better to have pkgpath still be a flag, which defaults to the value in gno.mod if not set.
My rationale: gnokey is primarily a tool to interact with the gno.land blockchain, and while gno.mod is a very important tool in local development, it is not essential to publish a package. So, in the way I see it, gnokey should essentially be the bare tool to create a transaction, with sane defaults like this one, but without asserting that gno.mod is a requirement.
What do you think?
Sorry for the delay in reply as well.
I agree 💯 (but...)
My vision is to make gno.mod more inclusive. Let's take an example of the versioning PR #1631, which will also make |
|
@harry-hov |
|
@zivkovicmilos last I remember, it was ready for review. Seems like now i need to fix merge conflicts. Will do this soon! |
harry-hov
force-pushed
the
hariom/addpkg-respect-gnomod
branch
from
a4260fc
to
d14ea00
Compare
harry-hov
force-pushed
the
hariom/addpkg-respect-gnomod
branch
from
d14ea00
to
e71b88f
Compare
BREAKING CHANGE: removed flag
-pkgPathfromaddpkgcommandSince PR #763 we are parsing pkg path from
gno.modfile while auto loading pkgs from /examples dir.Replicate the same behavior for
gnokey maketx addpkg ...Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the description