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: implement gno mod tidy #1035

Merged
merged 26 commits into from
Nov 6, 2023
Merged

Conversation

harry-hov
Copy link
Contributor

Contains initial implementation of gno mod tidy

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
  • Added new benchmarks to generated graphs, if any. More info here.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Aug 9, 2023
@harry-hov
Copy link
Contributor Author

harry-hov commented Aug 9, 2023

Blocked by: #955 (merged ✅ )

Blocked by: #1077 (merged ✅ )

@harry-hov harry-hov force-pushed the hariom/gno-mod-tidy branch from ece1e87 to 3adcfce Compare October 5, 2023 22:25
@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (89428c5) 47.84% compared to head (fe81089) 47.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1035      +/-   ##
==========================================
+ Coverage   47.84%   47.89%   +0.05%     
==========================================
  Files         369      369              
  Lines       62764    62836      +72     
==========================================
+ Hits        30028    30095      +67     
- Misses      30308    30316       +8     
+ Partials     2428     2425       -3     
Files Coverage Δ
gnovm/pkg/doc/dirs.go 92.70% <100.00%> (+6.71%) ⬆️
gnovm/pkg/gnomod/parse.go 54.26% <85.00%> (+7.04%) ⬆️
gnovm/cmd/gno/mod.go 83.42% <87.50%> (+2.69%) ⬆️

... and 5 files with indirect coverage changes

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

@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Oct 8, 2023
@harry-hov harry-hov force-pushed the hariom/gno-mod-tidy branch from 3959fda to bbd9c7d Compare October 8, 2023 22:21
@github-actions github-actions bot removed the 🧾 package/realm Tag used for new Realms or Packages. label Oct 8, 2023
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Oct 8, 2023
@harry-hov harry-hov added the 🌱 feature New update to Gno label Oct 9, 2023
@harry-hov harry-hov marked this pull request as ready for review October 10, 2023 09:24
@harry-hov harry-hov requested a review from a team as a code owner October 10, 2023 09:24
@harry-hov harry-hov requested a review from a team October 12, 2023 08:16
.github/workflows/examples.yml Outdated Show resolved Hide resolved
examples/Makefile Outdated Show resolved Hide resolved
gnovm/cmd/gno/mod.go Show resolved Hide resolved
}
for _, imp := range f.Imports {
importPath := strings.TrimPrefix(strings.TrimSuffix(imp.Path.Value, `"`), `"`)
if !strings.HasPrefix(importPath, "gno.land/") {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if !strings.HasPrefix(importPath, "gno.land/") {
if !strings.Contains(importPath, ".") {

I believe this is more future-proof

Copy link
Contributor Author

@harry-hov harry-hov Oct 16, 2023

Choose a reason for hiding this comment

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

Umm. I'm sure there are many place where "gno.land" is hard coded.

I'm okay with both, applying your changes here or to wait till we find a good solution and modify everywhere in a separate PR.

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I think right now we should only have "gno.land/*" import paths, while understanding that with IBC/ICS this will change and will allow for more domains, so that we should still treat all import paths which have a dot (before the first /) to come from external packages.

In any case, whichever is fine here. I've made a note on #1299.

gnovm/cmd/gno/mod_test.go Show resolved Hide resolved
gnovm/cmd/gno/mod_test.go Show resolved Hide resolved
@moul moul merged commit 4749369 into gnolang:master Nov 6, 2023
183 checks passed
harry-hov added a commit that referenced this pull request Nov 6, 2023
CI failing after merging #1035 
Seems like some mod files are not tidy.
gfanton pushed a commit to gfanton/gno that referenced this pull request Nov 9, 2023
Contains initial implementation of `gno mod tidy`

<details><summary>Contributors' checklist...</summary>

- [ ] 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
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Manfred Touron <[email protected]>
Co-authored-by: Morgan <[email protected]>
gfanton pushed a commit to gfanton/gno that referenced this pull request Nov 9, 2023
CI failing after merging gnolang#1035 
Seems like some mod files are not tidy.
moul added a commit to moul/gno that referenced this pull request Nov 14, 2023
Contains initial implementation of `gno mod tidy`

<details><summary>Contributors' checklist...</summary>

- [ ] 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
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Manfred Touron <[email protected]>
Co-authored-by: Morgan <[email protected]>
moul pushed a commit to moul/gno that referenced this pull request Nov 14, 2023
CI failing after merging gnolang#1035 
Seems like some mod files are not tidy.
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 🧾 package/realm Tag used for new Realms or Packages. 🌱 feature New update to Gno
Projects
Status: 🌟 Wanted for Launch
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants