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

feat: add gno version command #3002

Merged
merged 22 commits into from
Feb 5, 2025
Merged

Conversation

kazai777
Copy link
Contributor

I've added the version command to gno to get the version of gno that is installed

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests

@kazai777 kazai777 requested review from moul and thehowl as code owners October 22, 2024 23:06
@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Oct 22, 2024
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@jefft0 jefft0 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Oct 23, 2024
@jefft0
Copy link
Contributor

jefft0 commented Oct 23, 2024

Can you fix the failed CI test?

=== RUN   TestVersionApp/versiontestdata~gno_version~gopath_version.txtar
    main_test.go:129: 
        	Error Trace:	/home/runner/work/gno/gno/gnovm/cmd/gno/main_test.go:129
        	Error:      	Expected nil, but got: &errors.errorString{s:"unable to determine gno version"}
        	Test:       	TestVersionApp/versiontestdata~gno_version~gopath_version.txtar
        	Messages:   	err should be nil
=== RUN   TestVersionApp/versiontestdata~gno_version~git_version.txtar
    main_test.go:129: 
        	Error Trace:	/home/runner/work/gno/gno/gnovm/cmd/gno/main_test.go:129
        	Error:      	Expected nil, but got: &errors.errorString{s:"unable to determine gno version"}
        	Test:       	TestVersionApp/versiontestdata~gno_version~git_version.txtar
        	Messages:   	err should be nil
--- FAIL: TestVersionApp (0.00s)
    --- FAIL: TestVersionApp/versiontestdata~gno_version~gopath_version.txtar (0.00s)
    --- FAIL: TestVersionApp/versiontestdata~gno_version~git_version.txtar (0.00s)

@thehowl
Copy link
Member

thehowl commented Oct 23, 2024

Please, be aware of this -> #2755 (comment)

@kazai777
Copy link
Contributor Author

kazai777 commented Oct 29, 2024

Please, be aware of this -> #2755 (comment)

I've modified gno version so that the output is like the one you proposed in this issue #3005. The output is in this format [branch].[N]+[hash] or [tag] for when there is a specific tag pointing to the commit. Is this what you expect from the gno version command? @thehowl

Screenshot 2024-10-29 at 17 28 33

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

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

A few notes:

  • Instead of having this in cmd/gno, make a package gnovm/pkg/version, with var Version = "develop"
    • This will default to "develop" as a version, when somebody uses go install / go build directly.
    • It also allows us to eventually have this as gnoland version, too.
  • The ldflag should also go in the goreleaser set up: .github/goreleaser.yml

gnovm/Makefile Outdated Show resolved Hide resolved
@thehowl thehowl removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Oct 30, 2024
@thehowl thehowl marked this pull request as draft November 6, 2024 17:43
@thehowl
Copy link
Member

thehowl commented Nov 6, 2024

Marking draft for now while we wait for changes; put back into ready for review when changes are applied.

@kazai777
Copy link
Contributor Author

A few notes:

  • Instead of having this in cmd/gno, make a package gnovm/pkg/version, with var Version = "develop"

    • This will default to "develop" as a version, when somebody uses go install / go build directly.
    • It also allows us to eventually have this as gnoland version, too.
  • The ldflag should also go in the goreleaser set up: .github/goreleaser.yml

if I understand correctly, I create the version package in gnovm/pkg/version and only add the command in /cmd/gno/main.go ? @thehowl

@Gno2D2
Copy link
Collaborator

Gno2D2 commented Jan 13, 2025

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
  • The pull request description provides enough details (checked by @thehowl)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. 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:
  1. 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
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: kazai777/gno)

Then

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

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 At least 1 user(s) of the organization reviewed the pull request (with state "APPROVED")
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

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
The pull request description provides enough details

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors)
    └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])

Can be checked by

  • team core-contributors

@thehowl
Copy link
Member

thehowl commented Jan 13, 2025

if I understand correctly, I create the version package in gnovm/pkg/version and only add the command in /cmd/gno/main.go ?

yes (though it likely needs a new file in cmd/gno, like most commands)

@kazai777
Copy link
Contributor Author

I've modified the implementation of gno version command, so that the version is develop in the case of installation via go build/go install by creating a version package with a develop version by default.

Installation with the Makefile

Screenshot 2025-01-13 at 19 38 54


Installation with go build/go install

Screenshot 2025-01-13 at 19 40 09

@kazai777 kazai777 marked this pull request as ready for review January 13, 2025 19:03
.github/goreleaser.yaml Outdated Show resolved Hide resolved
.github/goreleaser.yaml Outdated Show resolved Hide resolved
gnovm/Makefile Outdated Show resolved Hide resolved
gnovm/cmd/gno/version_test.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/version_test.go Outdated Show resolved Hide resolved
@kazai777 kazai777 requested a review from thehowl January 20, 2025 23:02
@leohhhn
Copy link
Contributor

leohhhn commented Feb 4, 2025

Hey @kazai777, can you fix the conflicts here? I've pinged @thehowl to take a look.

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

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

clean

@thehowl thehowl merged commit 0b76b0b into gnolang:master Feb 5, 2025
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants