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

fix(tm2/std)!: use snake_case JSON fields for MemFile and MemPackage #2019

Merged
merged 7 commits into from
Jun 26, 2024

Conversation

thehowl
Copy link
Member

@thehowl thehowl commented May 2, 2024

This PR changes the MemFile and MemPackage to use snake_case fields for their JSON key names. This matches what all other objects in transactions do. A new test ensures that no transaction objects within the VM keeper have uppercase runes within their exported json fields.

BREAKING CHANGE: old m_addpkg and m_run transactions will stop working. Clients will need to update accordingly, and this change needs to be synced like #1939.

cc/ @zivkovicmilos @gnolang/berty @gnolang/onbloc

@github-actions github-actions bot added 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels May 2, 2024
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.68%. Comparing base (49fab4a) to head (5b426a5).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2019      +/-   ##
==========================================
+ Coverage   54.67%   54.68%   +0.01%     
==========================================
  Files         583      583              
  Lines       78502    78503       +1     
==========================================
+ Hits        42923    42933      +10     
+ Misses      32371    32366       -5     
+ Partials     3208     3204       -4     
Flag Coverage Δ
contribs/gnodev 23.81% <ø> (ø)
contribs/gnofaucet 14.46% <ø> (-0.86%) ⬇️
contribs/gnokeykc 0.00% <ø> (ø)
contribs/gnomd 0.00% <ø> (ø)
gno.land 62.54% <ø> (ø)
tm2 54.38% <ø> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@thehowl thehowl added the breaking change Functionality that contains breaking changes label May 2, 2024
@thehowl thehowl added this to the 🏗4️⃣ test4.gno.land milestone May 2, 2024
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

8pzaxm

gno.land/pkg/sdk/vm/package_test.go Show resolved Hide resolved
@zivkovicmilos
Copy link
Member

Let me coordinate a change on the JS clients before we merge it out 🫡

@zivkovicmilos
Copy link
Member

@thehowl can you merge in the master changes?

@thehowl thehowl requested a review from gfanton as a code owner May 29, 2024 14:02
@thehowl
Copy link
Member Author

thehowl commented May 29, 2024

@zivkovicmilos done!

@zivkovicmilos
Copy link
Member

I opened up an analogue PR for the JS clients:
gnolang/gno-js-client#132

@jinoosss
Copy link
Member

I opened a PR for Adena to support the changes.
onbloc/adena-wallet#522

@zivkovicmilos zivkovicmilos merged commit 3901e7e into master Jun 26, 2024
83 checks passed
@zivkovicmilos zivkovicmilos deleted the dev/morgan/memfile-package-snake-case branch June 26, 2024 13:43
@zivkovicmilos
Copy link
Member

For historical record:

We released gno-js-client v1.3.0 which houses these changes, so this is the earliest version to use with the master branch onwards

gfanton pushed a commit to gfanton/gno that referenced this pull request Jul 23, 2024
…nolang#2019)

This PR changes the MemFile and MemPackage to use `snake_case` fields
for their JSON key names. This matches what all other objects in
transactions do. A new test ensures that no transaction objects within
the VM keeper have uppercase runes within their exported json fields.

**BREAKING CHANGE:** old `m_addpkg` and `m_run` transactions will stop
working. Clients will need to update accordingly, and this change needs
to be synced like gnolang#1939.

cc/ @zivkovicmilos @gnolang/berty @gnolang/onbloc

---------

Co-authored-by: Miloš Živković <milos.zivkovic@tendermint.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Functionality that contains breaking changes 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants