-
Notifications
You must be signed in to change notification settings - Fork 2k
scripts/with-go-mod.sh: use symlink instead of -modfile #6039
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6039 +/- ##
=======================================
Coverage 58.91% 58.91%
=======================================
Files 358 358
Lines 29988 29988
=======================================
Hits 17666 17666
Misses 11341 11341
Partials 981 981 🚀 New features to boost your workflow:
|
scripts/vendor
Outdated
@@ -31,7 +31,7 @@ outdated() { | |||
echo "go-mod-outdated not found. Install with 'go install github.com/psampaz/[email protected]'" | |||
exit 1 | |||
fi | |||
(set -x ; go list -mod=vendor -mod=readonly -modfile=vendor.mod -u -m -json all | go-mod-outdated -update -direct) | |||
(set -x ; go list -mod=vendor -mod=readonly -u -m -json all | go-mod-outdated -update -direct) |
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.
go list -mod=vendor -mod=readonly
will just be -mod=readonly
right? 🤔
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.
Good one; I recall -mod=vendor
was needed to make it actually use vendor nowadays? Will need look them up 😂
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.
Oh! yes, you're right -mod
twice, I guess that's not combined, but overrides? (likely?)
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.
Yeah, it's like -tags
which is one that feels like it should combine but it overwrites (standard behavior of the stdlib flag
library).
scripts/with-go-mod.sh
Outdated
pushd "${ROOTDIR}" > /dev/null | ||
trap 'popd > /dev/null' RETURN |
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.
Whoa whoa whoa, what? This is wild, and unnecessary -- there are either flags on ln
that can DTRT or cp --symbolic-link
. If you can't get the arguments finagled correctly, let me know and I'll happily help sort it out further.
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.
❤️ I ran into the script not detecting that my own (relative) symlink was same as the symlink it wanted to create, so thought that changing to the directory would be the best way to "create a relative symlink here' and make it detect that my own symlink was "same" (so no warning printed) 🙈
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.
So maybe something like https://superuser.com/a/196655/101945? 👀
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.
OK; I think I found why it originally didn't work; I used an absolute path for both sides; now using absolute path for only the "source" side, and that looks to work; creates relative symlink, and detects my own symlink for being "ok"
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.
Oh! GitHub didn't show your comment; that looks interesting as well yes; let me give that a try as well!
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.
If you use absolute path on both sides, you can use ln --relative
to have ln
automatically "fix" it for you.
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.
Yeah; so ... macOS and BSD 🙈 - don't think they have that option 🫠
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.
While the "-modfile=vendor.mod" is slightly more correct, using a symlink allows for most of the go tools to work "as usual", just without an acual `go.mod` being committed in the repository. Signed-off-by: Sebastiaan van Stijn <[email protected]>
634f9d8
to
70a1d80
Compare
Make sure the symlinks point to the same file; not if both are identical (absolute or relative symlink); it's only for suppressing the warning so not critical. Signed-off-by: Sebastiaan van Stijn <[email protected]>
While the "-modfile=vendor.mod" is slightly more correct, using a symlink allows for most of the go tools to work "as usual", just without an acual
go.mod
being committed in the repository.- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)