Skip to content

setup nut for dependency management#331

Merged
technoweenie merged 15 commits intomasterfrom
nut
May 28, 2015
Merged

setup nut for dependency management#331
technoweenie merged 15 commits intomasterfrom
nut

Conversation

@technoweenie
Copy link
Contributor

Trying Nut out. I like the simpler config file. Unfortunately, it doesn't seem to be rewriting the imports for me. owenthereal/nut#24

@technoweenie
Copy link
Contributor Author

This is ready to go. Changes:

  • All 3rd party imports in the Git LFS code and the vendored libs should have a "github.com/github/git-lfs/vendor/nut/" prefix.
  • Somehow the "github.com/kr/pty" package was never vendored. It is now.
  • The various scripts are now much simpler. Should work with vanilla go commands, even on Windows.
  • Forked "github.com/bmizerany/assert" to fix an import statement in an example script. This gets around this issue.
  • There are references to "github.com/ogier/pflag". I think we need to clean up some references in some of spf13's packages upstream.

Why nut? Mostly because it worked out of the box. Godep didn't work quite right, and required a hack to get around an issue. We've also had great interactions with Nut's author, @jingweno in past Go projects. He mentioned an intent to support the suggested Go import conventions with a community specified vendor file format.

@sinbad
Copy link
Contributor

sinbad commented May 28, 2015

Looks fine to me. I never had the issue you encountered with godep (didn't have any platform-specific imports) and found it worked fine with updates over time - I see nut update isn't implemented yet - but it's all much the same. Main thing is the rewriting of the imports so it'll build out of the box which is a big plus.

@rubyist
Copy link
Contributor

rubyist commented May 28, 2015

Looks good, a couple notes:

Right now nut doesn't have an option to just rewrite imports (owenthereal/nut#8). This means manually typing the full vendored import path, and tools like goimports will no longer be able to help with that. If nut had this, for example, that could be part of the editor's formatting command (or script/fmt), run after goimports. This isn't a deal breaker, just something we might trip over.

Updating support would be nice, but working around that is pretty much what we had to do with gpm, so not a big deal (actually seems a little easier, I always had problems with gpm and .git dirs).

Running nut install in a freshly checked out version of this branch I get this every time:

diff --git a/vendor/_nuts/github.com/olekukonko/ts/doc.go b/vendor/_nuts/github.com/olekukonko/ts/doc.go
index d73c57e..50c63ca 100644
--- a/vendor/_nuts/github.com/olekukonko/ts/doc.go
+++ b/vendor/_nuts/github.com/olekukonko/ts/doc.go
@@ -20,7 +20,7 @@ Example

        import (
                "fmt"
-               "github.com/github/git-lfs/vendor/_nuts/github.com/olekukonko/ts"
+               "github.com/olekukonko/ts"
        )

        func main() {

This is code in a commented block, I'm thinking it got rewritten on accident and nut is just putting it back to normal, though I would think that no tool should be rewriting imports in comments. I think it should be safe to put that back as the normal import so we don't have to fix it every time we run nut.

@technoweenie
Copy link
Contributor Author

@rubyist: Ah, that might've been rewritten by me originally, and nut is just fixing it. I didn't realize it was in a giant comment block.

@rubyist
Copy link
Contributor

rubyist commented May 28, 2015

I went back and looked at the commits and realized that's probably what happened 👍

technoweenie added a commit that referenced this pull request May 28, 2015
setup nut for dependency management
@technoweenie technoweenie merged commit 47f7350 into master May 28, 2015
@technoweenie technoweenie deleted the nut branch May 28, 2015 20:42
This was referenced May 28, 2015
@owenthereal
Copy link

Thanks for using nut! I'll prioritize owenthereal/nut#8 higher since quite a few people ask for it.

andyneff added a commit to andyneff/git-lfs that referenced this pull request May 29, 2015
andyneff added a commit to andyneff/git-lfs that referenced this pull request May 30, 2015
@technoweenie technoweenie mentioned this pull request Jun 11, 2015
38 tasks
@ebramanti
Copy link

@technoweenie How do you fix the assert issue? I saw your fork but I'm not sure how to integrate!

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.

5 participants