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(cmd/gno): add support for gno.mod projects in gno doc #829

Merged
merged 15 commits into from
Jun 19, 2023

Conversation

thehowl
Copy link
Member

@thehowl thehowl commented May 18, 2023

Part of #522.

Feature is ready for review; will add some tests to check the functionality in all edge cases probably tomorrow.

Description

The gno doc command now understands local directories which use gno.mod.

One of the caveats is that the command will likely always complains that it wants a --root-dir variable; but I think we'll need to find a system-wide solution for this eventually (ie. when gno comes out of the first phase of development, and users install it in /usr/lib/gno, this should default to that directory).

asciicast

(Sorry for the small font, I still had my terminal in full-screen...)

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

@thehowl thehowl self-assigned this May 18, 2023
@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label May 18, 2023
@thehowl
Copy link
Member Author

thehowl commented May 18, 2023

It would seem like tests are failing because they can't reach test3?

Should we change gno mod tests to not use the network? Shall I open an issue for that?

@harry-hov
Copy link
Contributor

harry-hov commented May 18, 2023

Should we change gno mod tests to not use the network? Shall I open an issue for that?

Please. I was thinking about that for a long time. Maybe it's a good idea if we can start local test server for testing.

@thehowl thehowl marked this pull request as ready for review May 19, 2023 16:33
@thehowl thehowl requested a review from a team as a code owner May 19, 2023 16:33
@thehowl
Copy link
Member Author

thehowl commented May 19, 2023

It looks like it was a bug actually, but still I think tests should not rely on the network. I'll make an issue; I've added tests so this is now ready for review :)

gnovm/pkg/doc/dirs_test.go Show resolved Hide resolved
Copy link
Contributor

@harry-hov harry-hov left a comment

Choose a reason for hiding this comment

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

I left some comments. Overall solid work 💯

gnovm/pkg/doc/dirs.go Outdated Show resolved Hide resolved
gnovm/pkg/doc/dirs.go Outdated Show resolved Hide resolved
gnovm/pkg/doc/dirs.go Outdated Show resolved Hide resolved
gnovm/pkg/doc/dirs.go Outdated Show resolved Hide resolved
gnovm/cmd/gno/doc.go Outdated Show resolved Hide resolved
@thehowl thehowl requested a review from harry-hov June 19, 2023 14:10
@harry-hov harry-hov merged commit f51ce54 into gnolang:master Jun 19, 2023
44 checks passed
Doozers pushed a commit to Doozers/gno that referenced this pull request Aug 31, 2023
* feat(cmd/gno): add support for gno.mod projects in gno doc

* fix tests

* remove bfsRootDir

* update compat doc

* add tests

* harioms pr merged, remove duplicate code

* fix: tests for wrong arguments passed to newDirs

* xx

* fix dirsempty

* Apply suggestions from code review

Co-authored-by: Hariom Verma <hariom18599@gmail.com>

* s/tryParse/parse/g

---------

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
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: No status
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants