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

chore: relocate tm2/std.MemFile in gnovm/std #2910

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

moul
Copy link
Member

@moul moul commented Oct 5, 2024

This PR removes "gno.land" from all tm2/.../*.go files.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul moul self-assigned this Oct 5, 2024
@github-actions github-actions bot added 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Oct 5, 2024
@moul moul changed the title dev/moul/mv memfile chore: relocate tm2/std.MemFile in gnovm/std Oct 5, 2024
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.95%. Comparing base (d3049ae) to head (40eaad0).

Files with missing lines Patch % Lines
gno.land/pkg/keyscli/run.go 66.66% 1 Missing ⚠️
gno.land/pkg/sdk/vm/keeper.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2910      +/-   ##
==========================================
- Coverage   60.97%   60.95%   -0.03%     
==========================================
  Files         564      565       +1     
  Lines       75273    75288      +15     
==========================================
- Hits        45901    45889      -12     
- Misses      26005    26029      +24     
- Partials     3367     3370       +3     
Flag Coverage Δ
contribs/gnodev 61.46% <ø> (ø)
contribs/gnofaucet 15.31% <ø> (ø)
gno.land 67.92% <77.77%> (ø)
gnovm 65.79% <100.00%> (+0.01%) ⬆️
misc/genstd 80.54% <ø> (ø)
misc/logos 20.23% <ø> (ø)
tm2 62.05% <100.00%> (+0.02%) ⬆️

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.

@moul
Copy link
Member Author

moul commented Oct 5, 2024

I chose to create gnovm/pkg/std, but we should consider merging it with gnovm/pkg/stdlibs/std. What do you think?

I removed the .pb.go files that were added forcefully. Should I revert this change?

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
@thehowl
Copy link
Member

thehowl commented Oct 5, 2024

I have a more general question - why do we name anything std? Is there any standard involved?

@moul moul marked this pull request as ready for review October 5, 2024 10:05
@moul
Copy link
Member Author

moul commented Oct 5, 2024

I have a more general question - why do we name anything std? Is there any standard involved?

Right now, we refer to these as "lowest level types." The closest name we have is "sys," and the system contracts are currently named "r/sys." We also considered naming them "r/std." Essentially, this serves as our standard for low-level system components.

In this repository, we have three standard packages designed to be imported to connect with the components without loading other parts (lightweight, no side effects).

An alternative for the Go side would be to rename them to "types." However, it's important to distinguish between "system/low-level types" and "all types." One invites connection with a minimal API (communication layer), while the other is more internal and advanced.

I'm open to reconsidering a renaming, although I believe we will likely prefer to stick with the current approach. My question is about balancing independence with unifying gnovm's standard messages.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul
Copy link
Member Author

moul commented Oct 5, 2024

By the way, the relocation is complete, and the CI is green. I have two other PRs in mind that will depend on merging this one. Please feel free to review it, and we can discuss the "naming" topic later, maybe.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul moul mentioned this pull request Oct 5, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: No status
Status: 📥 Inbox
Status: Triage
Development

Successfully merging this pull request may close these issues.

2 participants