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

lib: Move to go modules with regret #671

Closed
wants to merge 1 commit into from

Conversation

purpleidea
Copy link
Owner

I'm incredibly annoyed that go modules are being forced onto us, as the
old system with vendor/ and git submodules worked great. Unfortunately,
so much FUD around git submodules, even by well-known, highly paid
engineers has caused this solution to be deemed most unacceptable.

So much for the golang compatibility promise-- turns out it doesn't
apply to the useful parts that we actually care about like this.

Thanks to frebib for his help with this suffering.

Tips:

  • please read the style guide before submitting your patch:
    docs/style-guide.md

  • commit message titles must be in the form:

topic: Capitalized message with no trailing period

or:

topic, topic2: Capitalized message with no trailing period

  • golang code must be formatted according to the standard, please run:
make gofmt		# formats the entire project correctly

or format a single golang file correctly:

gofmt -w yourcode.go
  • please rebase your patch against current git master:
git checkout master
git pull origin master
git checkout your-feature
git rebase master
git push your-remote your-feature
hub pull-request	# or submit with the github web ui
  • after a patch review, please ping @purpleidea so we know to re-review:
# make changes based on reviews...
git add -p		# add new changes
git commit --amend	# combine with existing commit
git push your-remote your-feature -f
# now ping @purpleidea in the github PR since it doesn't notify us automatically

Thanks for contributing to mgmt and welcome to the team!

I'm incredibly annoyed that go modules are being forced onto us, as the
old system with vendor/ and git submodules worked great. Unfortunately,
so much FUD around git submodules, even by well-known, highly paid
engineers has caused this solution to be deemed most unacceptable.

So much for the golang compatibility promise-- turns out it doesn't
apply to the useful parts that we actually care about like this.

Thanks to frebib for his help with this suffering.
@purpleidea
Copy link
Owner Author

@frebib

@frebib
Copy link
Contributor

frebib commented Sep 27, 2021

https://github.com/frebib/mgmt/commits/feat/go-mod-fixes
Here are my fixes. Please look over the last commit as I removed anything that seemed no longer applicable. Feel free to exclude any of my removals as you see fit, then hopefully we can get this merged

@frebib
Copy link
Contributor

frebib commented Sep 27, 2021

You're also going to want to remove the rest of .gitmodules because any partial vendor directory will (probably) confuse go-mod. It'll either be ignored outright or cause build failures depending on how you invoke go build -mod=???
Updated my branch

@purpleidea
Copy link
Owner Author

Nice, you rebased on top. Fantastic. Will look later tonight.

@frebib
Copy link
Contributor

frebib commented Sep 27, 2021

I found that with go modules, gometalinter fails to install, for some reason. You may need this commit to make the tests pass
frebib@8fde6aa

Program: program,
Version: version,
Copying: copying,
Flags: flags,
Copy link
Contributor

Choose a reason for hiding this comment

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

We need #670 finished and merged. It would have picked this up!

main.go:52: File is not `goimports`-ed (goimports)
		Flags: flags,
diff --git a/main.go b/main.go
index 8eba6fc..8c36be1 100644
--- a/main.go
+++ b/main.go
@@ -49,7 +49,7 @@ func main() {
                Program: program,
                Version: version,
                Copying: copying,
-               Flags: flags,
+               Flags:   flags,
        }
        if err := mgmt.CLI(cliArgs); err != nil {
                fmt.Println(err)

@purpleidea purpleidea closed this Sep 29, 2021
@purpleidea purpleidea deleted the feat/go-mod-without-consent branch September 29, 2021 02:21
@purpleidea
Copy link
Owner Author

Replaced by #674

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants