refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller and extend abbreviations #3374

hthieu1110 · 2024-12-19T14:29:39Z

Linked to #1475

GetCallerAt =>CallerAt
GetOrigSend => OriginSend
origSend => originSend
GetOrigCaller => OriginCaller
origCaller => originCaller
Orig => Origin
Addr => Address
PrevRealm => PreviousRealm

This is a BREAKING CHANGE.

…efix-1475

Gno2D2 · 2024-12-19T14:31:16Z

🛠 PR Checks Summary

🔴 Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff

Manual Checks (for Reviewers):

IGNORE the bot requirements for this PR (force green CI check)
Determine if infra needs to be updated before merging

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🔴 Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff

☑️ Contributor Actions:

Fix any issues flagged by automated checks.
Follow the Contributor Checklist to ensure your PR is ready for review.
- Add new tests, or document why they are unnecessary.
- Provide clear examples/screenshots, if necessary.
- Update documentation, if required.
- Ensure no breaking changes, or include BREAKING CHANGE notes.
- Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 The pull request was created from a fork (head branch repo: hthieu1110/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff

If

🟢 Condition met
└── 🟢 A changed file matches this pattern: ^docs/ (filename: docs/assets/how-to-guides/creating-grc20/mytoken-1.gno)

Then

🔴 Requirement not satisfied
└── 🔴 Or
    ├── 🔴 And
    │   ├── 🔴 Pull request author is a member of the team: devrels
    │   └── 🔴 At least 1 user(s) of the team tech-staff approved pull request
    └── 🔴 And
        ├── 🔴 Pull request author is a member of the team: tech-staff
        └── 🟢 At least 1 user(s) of the team devrels approved pull request

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)

If
🟢 Condition met
└── 🟢 On every pull request
Can be checked by

Any user with comment edit permission

Determine if infra needs to be updated before merging

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: master
    └── 🟢 Or
        ├── 🔴 A changed file matches this pattern: Dockerfile
        ├── 🟢 A changed file matches this pattern: ^misc/deployments (filename: misc/deployments/test5.gno.land/genesis_txs.jsonl)
        ├── 🔴 A changed file matches this pattern: ^misc/docker-
        ├── 🔴 A changed file matches this pattern: ^.github/workflows/releaser.*\.yml$
        └── 🔴 A changed file matches this pattern: ^.github/workflows/portal-loop\.yml$

Can be checked by

team devops

codecov · 2024-12-19T14:59:18Z

Codecov Report

Attention: Patch coverage is 55.55556% with 16 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gnovm/tests/stdlibs/std/std.go	40.00%	6 Missing ⚠️
gnovm/stdlibs/std/banker.go	0.00%	5 Missing ⚠️
gnovm/stdlibs/std/native.go	50.00%	5 Missing ⚠️

📢 Thoughts on this report? Let us know!

MikaelVallenet · 2024-12-20T10:10:13Z

docs/reference/stdlibs/std/realm.md

@@ -22,7 +22,7 @@ Returns the **Address** field of the realm it was called upon.

 #### Usage


missing modification in struct + subtitle name func

MikaelVallenet · 2024-12-20T10:11:15Z

docs/reference/stdlibs/std/chain.md

 ```
 ---

-## GetOrigPkgAddr
+## GetOriginPkgAddr


GetOriginPkgAddress

MikaelVallenet · 2024-12-20T10:12:29Z

docs/reference/stdlibs/std/testing.md

 ```
 ---

-## TestSetOrigPkgAddr
+## TestSetOriginPkgAddr


Why you "Addr" and sometimes "Address" which one is good, we should try to keep it consistent

Address is good :) this is the missing refactor

MikaelVallenet · 2024-12-20T10:13:38Z

examples/gno.land/p/demo/grc/grc20/examples_test.gno

@@ -7,7 +7,7 @@ func ExampleExposeBankForMaketxRunOrImports() {}
 func ExampleCustomTellerImpl()                {}
 func ExampleAllowance()                       {}
 func ExampleRealmBanker()                     {}
-func ExamplePrevRealmBanker()                 {}
+func ExamplePreviousRealmBanker()                 {}


should we limit the scope of the PR to change naming in std lib only

This is related to Realm so I think we should change also here

…efix-1475

notJoon · 2024-12-26T05:20:29Z

While changing the Get prefix from std APIs aligns with Go's conventions. I am concerned that an immediate change would severely impact backward compatibility for other projects.

Taking Gnoswap as an example, once a contract is deployed, it becomes practically impossible to modify¹. This means there would be no way to adapt if these method names change, which could lead to fatal issues in production evironments.

Therefore, I propose the following gradual approach:

Mark existing APIs as deprecated while maintaining them
Add new methods following the new naming convention
Gradually phase out support for existing methods after providing sufficient migration period

This approach would allow existing projects adequate time to apply to the new changes.

Also, I think this PR must mark as a breaking change BTW.

While this task could be executed through governance voting, we would have no way to respond or adapt during that voting period. ↩

leohhhn

Looks good to me 👍

..

@notJoon

This is part of the std refactor that we're in the works for, for which we cannot avoid breaking backwards compatibility unfortunately.

chore: remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller

c57797d

github-actions bot assigned hthieu1110 Dec 19, 2024

github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Dec 19, 2024

Merge remote-tracking branch 'remote/master' into chore/remove-get-pr…

6156309

…efix-1475

Gno2D2 requested review from a team December 19, 2024 14:31

hthieu1110 added 2 commits December 19, 2024 21:39

chore: expanding abbreviations

c58ae72

chore: remove prefix GetCallerAt => CallerAt

6879236

hthieu1110 marked this pull request as ready for review December 19, 2024 16:01

hieu.ha added 5 commits December 20, 2024 11:07

Merge branch 'master' into chore/remove-get-prefix-1475

d41482e

refactor: PrevRealm => PreviousRealm

fbc982d

refactor: Orig => Origin

0f79638

refactor: orig => origin

627d000

refactor: Addr => Address

8489ec4

hthieu1110 changed the title ~~feat(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller~~ refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller Dec 20, 2024

hthieu1110 changed the title ~~refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller~~ refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller and extend abbreviations Dec 20, 2024

hieu.ha added 2 commits December 20, 2024 13:33

refactor: refactor missing Addr => Address for Realm

e2223ff

fix: fix test crossrealm

eb965f1

jefft0 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Dec 20, 2024

MikaelVallenet reviewed Dec 20, 2024

View reviewed changes

Kouteki added the in focus Core team is prioritizing this work label Dec 20, 2024

Kouteki requested review from petar-dambovaliev and mvertes and removed request for a team December 20, 2024 12:07

Kouteki added this to the 🚀 Mainnet beta launch milestone Dec 20, 2024

Gno2D2 requested review from a team December 20, 2024 12:07

hthieu1110 added 4 commits December 20, 2024 20:00

refactor: address feedbacks

ad57bfc

Merge remote-tracking branch 'remote/master' into chore/remove-get-pr…

afd55ca

…efix-1475

Merge remote-tracking branch 'remote/master' into chore/remove-get-pr…

df80fe8

…efix-1475

refactor: refactor new merged code

2a4eb54

leohhhn approved these changes Dec 27, 2024

View reviewed changes

Gno2D2 requested a review from a team December 27, 2024 03:52

Merge branch 'master' into chore/remove-get-prefix-1475

90b9a33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller and extend abbreviations #3374

refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller and extend abbreviations #3374

hthieu1110 commented Dec 19, 2024 •

edited by leohhhn

Loading

Gno2D2 commented Dec 19, 2024 •

edited

Loading

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Then

If

Then

If

Can be checked by

If

Can be checked by

codecov bot commented Dec 19, 2024 •

edited

Loading

MikaelVallenet Dec 20, 2024

hthieu1110 Dec 20, 2024

MikaelVallenet Dec 20, 2024

hthieu1110 Dec 20, 2024

MikaelVallenet Dec 20, 2024

hthieu1110 Dec 20, 2024

hthieu1110 Dec 20, 2024

MikaelVallenet Dec 20, 2024

hthieu1110 Dec 20, 2024

hthieu1110 Dec 20, 2024

notJoon commented Dec 26, 2024 •

edited

Loading

leohhhn left a comment •

edited

Loading

		@@ -22,7 +22,7 @@ Returns the Address field of the realm it was called upon.

		#### Usage

refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller and extend abbreviations #3374

Are you sure you want to change the base?

refactor(gnovm): remove Get prefixes from GetCallerAt, GetOrigSend, GetOrigCaller and extend abbreviations #3374

Conversation

hthieu1110 commented Dec 19, 2024 • edited by leohhhn Loading

Gno2D2 commented Dec 19, 2024 • edited Loading

🛠 PR Checks Summary

Manual Checks (for Reviewers):

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Then

If

Then

If

Can be checked by

If

Can be checked by

codecov bot commented Dec 19, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

notJoon commented Dec 26, 2024 • edited Loading

Footnotes

leohhhn left a comment • edited Loading

Choose a reason for hiding this comment

hthieu1110 commented Dec 19, 2024 •

edited by leohhhn

Loading

Gno2D2 commented Dec 19, 2024 •

edited

Loading

codecov bot commented Dec 19, 2024 •

edited

Loading

notJoon commented Dec 26, 2024 •

edited

Loading

leohhhn left a comment •

edited

Loading