Add a Go Plugin README.

Review Request #3866 — Created May 11, 2016 and submitted

jsirois
pants
jsirois/issues/2482/go-contrib-readme
2482, 3402
3881
494a80c...
pants-reviews
benjyw, lahosken, stuhood
contrib/go/README.md | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 186 insertions(+)
The README is rendered here:
  https://github.com/jsirois/pants/blob/jsirois/issues/2482/go-contrib-readme/contrib/go/README.md

CI went green here:
  http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3402/15/
  • 0
  • 0
  • 5
  • 0
  • 5
Description From Last Updated
JS
JS
JS
LA
  1. As someone who edits Go with an Emacs mode that's 95% gofmt compatible, I ask: Is there a Pants-ish way to gofmt my code? Or do I want to do that outside of Pants?
    1. The `./pants go` section is meant to cover this, although it uses `go list` as an example and not `go fmt`.  I'll add a hat tip to `go fmt` without a full example.
      IOW, run this in pants and see 1 set of edits to files that make sense to be checked in with bad style: `./pants go contrib/go/examples/src/go/:: -- fmt ./...`
    2. That section was weak on the explanation on re-read.  Added a sentence about the general escape-hatch use case and did add a highlight of the `go fmt` case since I suspect you are right to ask about this likely super-common task/question.
  2. contrib/go/README.md (Diff revision 1)
     
     

    Do I want to run this once from the top of my pants dir tree? Or run it a few times, once each in some dir or other? Or...?

    1. Once at the top of the go tree, I'll clarify.
  3. 
      
JS
  1. OK - hands off from me now - this is good to go.
  2. 
      
JS
JS
JS
BE
  1. Thanks for doing this! It's very clear. I have a couple of comments, and I'll follow up soon after I try all this out in my go repo. :)

  2. contrib/go/README.md (Diff revision 5)
     
     

    I suggest adding a comma after "vendoring". I initially read the "without" as referring to "vendoring and other utilities".

  3. contrib/go/README.md (Diff revision 5)
     
     

    I would put this under a separate "maintenance" heading, to emphasize that your work is not done after the initial setup.

  4. 
      
YU
  1. 
      
    1. Thanks for writing this doc (expecially given that I am srtuggling with go plugin)! With my very limited (almost none) go knowledge, I think it looks good.

  2. contrib/go/README.md (Diff revision 5)
     
     

    missing "'" at the end of {{.Imports}}

  3. 
      
JS
YU
  1. Ship It!
  2. 
      
BE
  1. 
      
  2. contrib/go/README.md (Diff revision 6)
     
     

    This worked like a charm!

  3. contrib/go/README.md (Diff revision 6)
     
     

    This failed with the following error:

    16:58:25 00:00       [go.buildgen]
    16:59:17 00:52   [complete]
                   FAILURE
    Exception caught: (<type 'exceptions.OSError'>)
    
    Exception message: [Errno 2] No such file or directory: '<root>/src/go/foo/bar'
    

    This is probably because my existing src/go is organized like a standard Go project:

    $ ls src/go/
    bin pkg src
    

    So the actual sources are under src/go/src.

    Shouldn't we be able to support this? Pants generally doesn't care what directory structure you have under your source roots.

    1. If you can run with backtraces enabled and filean issue that would be wonderful.
      No clue what is going on from that log snip.
    2. Done, just wanted to see if there was something obviously wrong with what I was trying to do. See https://github.com/pantsbuild/pants/issues/3417. Thanks!

    3. I couldn't repro your issue using this: https://github.com/pantsbuild/issues-3417
      Hopefully the differences between your real repo and that distilled one will be apparent and help narrow down the issue.
      
      The steps did highlight a compile-time only (only compile does transitive 3rdparty resolves, buildgen does not) directed seed step not represented in this README. I'll add that to this doc for completeness.
    4. Docs updated with source root configuration procedure as well as transitive dependency fixing.
  4. 
      
JS
DB
  1. 
      
  2. contrib/go/README.md (Diff revision 6)
     
     

    I added checkstyle support in https://github.com/pantsbuild/pants/commit/1dfed202cea4a16a7418030b713b375e9a5bb7c1 and it lives in https://github.com/pantsbuild/pants/blob/master/contrib/go/src/python/pants/contrib/go/tasks/go_checkstyle.py

    Is there something different than most people want? It's been working well for us, fwiw.

    1. Aha - thanks for pointing that out.  I'll change the note to point out pants checks your go format, but doesn't help you fix it as the lead in to the command example.
    2. Fixed.
  3. 
      
BE
  1. It might be worth addressing the question of how Pants detects that a go dep is "remote", and how it fetches it.

    I'm currently attempting to modify [archive-fetcher] --matchers because I have a dep that isn't in one of the 4 _DEFAULT_MATCHERS, and it's non-trivial. I don't know if this is a common enough occurrence to go into in great detail in the README, but it's worth giving an overview, and mentioning that you may need to tinker with this if you encounter errors like this:

    ```Exception caught: (<type 'exceptions.OSError'>)

    Exception message: [Errno 2] No such file or directory: '<root>/src/go/nonstandard.repo/foo/bar/baz'
    ```

    1. I think that one I'll leave to you since you'll have fresh new-user insight.  I'd like to get this in without too much more scope creep.
    2. SGTM

  2. 
      
BE
  1. Ship It!
  2. 
      
JS
JS
BE
  1. Ship It!
    1. Not so fast! I'm still working on addressing your comment.
      This RB is a blocker since the expanded setup intstructions will include use of ./pants roots to determine if custom source roots need to be configured in pants.ini.

  2. 
      
DB
  1. Ship It!
  2. 
      
JS
JS
JS
  1. I'll submit this once CI goes green unless there is further feedback.
  2. 
      
JS
Review request changed

Status: Closed (submitted)

Change Summary:

Now on master:

git log -1 origin/master
commit a7424723e52262db17dc4c9ed724763f25c6dae2
Author: John Sirois <john.sirois@gmail.com>
Date:   Fri May 13 16:44:21 2016 -0600

    Add a Go Plugin README.
    
    Testing Done:
    The README is rendered here:
      https://github.com/jsirois/pants/blob/jsirois/issues/2482/go-contrib-readme/contrib/go/README.md
    
    CI went green here:
      http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3402/15/
    
    Bugs closed: 2482, 3402
    
    Reviewed at https://rbcommons.com/s/twitter/r/3866/
Loading...