-
Notifications
You must be signed in to change notification settings - Fork 70
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
refactor: Modualize the samples so that they can be gonew #130
Conversation
README.md
Outdated
|
||
```shell | ||
$ go install golang.org/x/tools/cmd/gonew@latest | ||
$ gonew github.com/ikawaha/examples/basic@latest github.com/<your_repo>/basic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be gonew goa.design/examples/basic@latest github.com/<your_repo>/basic
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooops, fixed in e0823af
tracing/go.mod
Outdated
go 1.21.3 | ||
|
||
// Temporarily replacing as the official release is not yet available | ||
replace goa.design/examples/basic => github.com/ikawaha/calc v0.0.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this replace directive is a temporary measure. It's in place because goa.design/examples/basic
currently doesn't support modules. Once it does, and an official release is available, we can remove this replacement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be okay to remove it in this PR, even if it might cause the tests to fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To address your feedback regarding the replace directive, I've made adjustments to ensure a smoother build process. Specifically, I've removed the tracing
sample from this PR (772a41a). Instead, I've prepared a separate PR specifically for adding the tracing
sample(ikawaha#1). This eliminates the need to push the noted replace directive. If it works for you, could we merge this PR first and then move on to merging the subsequent PR for tracing
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, this looks great.
I've updated the README. I've added information about the Goa Clue example and mentioned the use of the gonew command for easy cloning. d9ffbdc |
Thank you for the great contribution! |
see. #131
Modularized each sample using Go modules.
*Temporarily replaced the library with
goa.design/examples/basic
the tracing sample. This library replacement is temporary. After this PR is merged and released, we'll need to revert the replacement.Update README with gonew cloning instructions. f700e77