Page MenuHomePhabricator

Provide a CONTRIBUTING.md guide
ClosedPublic

Authored by dduvall on Feb 28 2018, 7:11 PM.

Details

Reviewers
thcipriani
demon
hashar
mmodell
zeljkofilipin
greg
Jrbranaa
Group Reviewers
Release-Engineering-Team
Commits
rGBLBR73ee493d6e54: Provide a CONTRIBUTING.md guide
Patch without arc
git checkout -b D993 && curl -L https://phabricator.wikimedia.org/D993?download=true | git apply
Summary

To help new developers get on board, let's provide a CONTRIBUTING.md
doc.

Test Plan

Proofread with your eyeballs.

Diff Detail

Repository
rGBLBR Blubber
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dduvall created this revision.Feb 28 2018, 7:11 PM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald TranscriptFeb 28 2018, 7:11 PM
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
dduvall requested review of this revision.Feb 28 2018, 7:11 PM
dduvall updated this revision to Diff 2602.Feb 28 2018, 7:22 PM

Added sections on review and landing

hashar requested changes to this revision.Feb 28 2018, 8:41 PM

I have applied the CONTRIBUTING tips from a scratch box running Debian Stretch :)

End result: arc unit --everything is all green *success*

CONTRIBUTING.md
13

nitpick: macOS :]

14

I went with 1.8 from stretch-backports. I guess testing (1.10) is better anyway.

52

On Debian golang-go does not provide godoc. One has to install the package golang-golang-x-tools but it is a snapshot from 2016 and I could not figure out how to make it work bah.

There is also go doc but hmm it does not quite work :]

For the newbies like me, maybe we would want to publish the doc on https://doc.wikimedia.org/ and simply point at it?


Ditto golint is an extra package.

57

...we've opted to commit a minimal vendor directory which contains all the needed dependencies. It has been automatically populated ...

66

+3 on explaining how to refresh dependencies and update the vendored package. That is going to be a nice time saver.

75
$ arc unit
No tests to run.
$ arc lint
No paths are lintable.

Eek?

77
$ go test ./...
../../go/src/phabricator.wikimedia.org/source/blubber/vendor/github.com/docker/distribution/blobs.go:10:2: cannot find package "github.com/docker/distribution/context" in any of:
	/home/hashar/go/src/phabricator.wikimedia.org/source/blubber/vendor/github.com/docker/distribution/context (vendor tree)
	/usr/lib/go-1.8/src/github.com/docker/distribution/context (from $GOROOT)
	/home/hashar/go/src/github.com/docker/distribution/context (from $GOPATH)
../../go/src/phabricator.wikimedia.org/source/blubber/vendor/github.com/stretchr/testify/doc.go:19:2: cannot find package "github.com/stretchr/testify/http" in any of:
	/home/hashar/go/src/phabricator.wikimedia.org/source/blubber/vendor/github.com/stretchr/testify/http (vendor tree)
	/usr/lib/go-1.8/src/github.com/stretchr/testify/http (from $GOROOT)
	/home/hashar/go/src/github.com/stretchr/testify/http (from $GOPATH)
../../go/src/phabricator.wikimedia.org/source/blubber/vendor/github.com/stretchr/testify/doc.go:21:2: cannot find package "github.com/stretchr/testify/mock" in any of:
	/home/hashar/go/src/phabricator.wikimedia.org/source/blubber/vendor/github.com/stretchr/testify/mock (vendor tree)
	/usr/lib/go-1.8/src/github.com/stretchr/testify/mock (from $GOROOT)
	/home/hashar/go/src/github.com/stretchr/testify/mock (from $GOPATH)
78

If arc unit supports filtering a specific function, we should give an example for it and probably could drop references to go test?

81

Can probably refer to golint as well.

This revision now requires changes to proceed.Feb 28 2018, 8:41 PM
thcipriani added inline comments.Mar 6 2018, 2:55 PM
CONTRIBUTING.md
14

Hrm, I'm running testing, but I'd prefer not making people run testing and just use backports; however, backports is still on 1.8 and we, I think, need 1.9 (see my other comment here).

77

Hrm, this looks like it means we may need to use golang 1.9 (see: https://golang.org/doc/go1.9#vendor-dotdotdot)

thcipriani added inline comments.Mar 6 2018, 2:59 PM
CONTRIBUTING.md
14

This might be what we want: https://github.com/purpleidea/mgmt/blob/master/docs/quick-start-guide.md#installing-golang

Except I'd change the verbiage "too old" to be "sensible enough not to try to keep pace with golang" :)

dduvall updated this revision to Diff 2648.Mar 20 2018, 8:14 PM
dduvall marked 5 inline comments as done.

Rebased and integrated feedback.

thcipriani accepted this revision.Mar 22 2018, 3:41 PM

Looks good to me! Nice work.

hashar accepted this revision.Mar 26 2018, 8:19 PM

All great! Thank you for the follows up.

This revision is now accepted and ready to land.Mar 26 2018, 8:19 PM
This revision was automatically updated to reflect the committed changes.