From 1072ec27b8372671d8f42ba75464dd23822147f5 Mon Sep 17 00:00:00 2001 From: Brandur Date: Mon, 18 Jun 2018 13:52:58 -0700 Subject: [PATCH] Activate linting for a few packages This patch brings golint [1] into the Travis build and adds a make target that will start running it on a few packages that we've expunged of all linting errors. I've just added a few packages to this initially because there are hundreds of linting errors across the entire system, and cleaning them all up would be an incredibly noisy diff. My recommendation is that we issue smaller patches with ~1-6 packages converted over at a time to make reviews easier (and more reliable), and add them to the whitelist in `Makefile` as we go. After all packages are converted, we change the call to `golint ./...`, and it'll always just run for everything. Golint's major advancement will be to help us vet that Godoc comments are formatted correctly, e.g. they all start with a function name like `Get retrieves a charge`, and that exported functions _have_ Godoc comments. This will hopefully get us to a place where we can generate and publish a complete set of clean Godocs. [1] https://github.com/golang/lint --- .travis.yml | 4 +++- Makefile | 13 ++++++++++++- account/client.go | 31 +++++++++++++++++++------------ charge/client.go | 33 ++++++++++++++++----------------- 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/.travis.yml b/.travis.yml index 55a26746f0..4b815467c1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,9 @@ before_install: + # The testify require framework is used for assertions in the test suite - go get -u github.com/stretchr/testify/require - # Install code coverage / coveralls tooling + # Install lint / code coverage / coveralls tooling + - go get -u golang.org/x/lint/golint - go get -u golang.org/x/tools/cmd/cover - go get -u github.com/modocache/gover - go get -u github.com/mattn/goveralls diff --git a/Makefile b/Makefile index 15e3748a42..0d23423114 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -all: test bench vet check-gofmt +all: test bench vet lint check-gofmt bench: go test -race -bench . -run "Benchmark" ./form @@ -9,6 +9,17 @@ build: check-gofmt: scripts/check_gofmt.sh +# We're trying to get linting activated, but there are so many errors that +# we're doing so incrementally instead of pushing it through as part of one +# giant patch. +# +# As new packages get cleaned up, add them here so that we don't regress. When +# all packages are here, switch to a `./...` instead of linting every package +# individually. +lint: + golint -set_exit_status ./account + golint -set_exit_status ./charge + test: go test -race ./... diff --git a/account/client.go b/account/client.go index 1e35e7f206..aa4cbdaedb 100644 --- a/account/client.go +++ b/account/client.go @@ -1,3 +1,6 @@ +// Package account provides API functions related to accounts. +// +// For more details, see: https://stripe.com/docs/api/go#accounts. package account import ( @@ -7,7 +10,7 @@ import ( "github.com/stripe/stripe-go/form" ) -// Client is used to invoke /account APIs. +// Client is used to invoke APIs related to accounts. type Client struct { B stripe.Backend Key string @@ -18,28 +21,31 @@ func New(params *stripe.AccountParams) (*stripe.Account, error) { return getC().New(params) } +// New creates a new account. func (c Client) New(params *stripe.AccountParams) (*stripe.Account, error) { acct := &stripe.Account{} err := c.B.Call(http.MethodPost, "/accounts", c.Key, params, acct) return acct, err } -// Get returns the details of an account. +// Get retrieves the authenticating account. func Get() (*stripe.Account, error) { return getC().Get() } +// Get retrieves the authenticating account. func (c Client) Get() (*stripe.Account, error) { account := &stripe.Account{} err := c.B.Call(http.MethodGet, "/account", c.Key, nil, account) return account, err } -// GetByID returns the details of your account. +// GetByID retrieves an account. func GetByID(id string, params *stripe.AccountParams) (*stripe.Account, error) { return getC().GetByID(id, params) } +// GetByID retrieves an account. func (c Client) GetByID(id string, params *stripe.AccountParams) (*stripe.Account, error) { path := stripe.FormatURLPath("/accounts/%s", id) account := &stripe.Account{} @@ -47,11 +53,12 @@ func (c Client) GetByID(id string, params *stripe.AccountParams) (*stripe.Accoun return account, err } -// Update updates the details of an account. +// Update updates an account. func Update(id string, params *stripe.AccountParams) (*stripe.Account, error) { return getC().Update(id, params) } +// Update updates an account. func (c Client) Update(id string, params *stripe.AccountParams) (*stripe.Account, error) { path := stripe.FormatURLPath("/accounts/%s", id) acct := &stripe.Account{} @@ -59,11 +66,12 @@ func (c Client) Update(id string, params *stripe.AccountParams) (*stripe.Account return acct, err } -// Del deletes an account +// Del deletes an account. func Del(id string, params *stripe.AccountParams) (*stripe.Account, error) { return getC().Del(id, params) } +// Del deletes an account. func (c Client) Del(id string, params *stripe.AccountParams) (*stripe.Account, error) { path := stripe.FormatURLPath("/accounts/%s", id) acct := &stripe.Account{} @@ -71,11 +79,12 @@ func (c Client) Del(id string, params *stripe.AccountParams) (*stripe.Account, e return acct, err } -// Reject rejects an account +// Reject rejects an account. func Reject(id string, params *stripe.AccountRejectParams) (*stripe.Account, error) { return getC().Reject(id, params) } +// Reject rejects an account. func (c Client) Reject(id string, params *stripe.AccountRejectParams) (*stripe.Account, error) { path := stripe.FormatURLPath("/accounts/%s/reject", id) acct := &stripe.Account{} @@ -83,11 +92,12 @@ func (c Client) Reject(id string, params *stripe.AccountRejectParams) (*stripe.A return acct, err } -// List lists your accounts. +// List returns an iterator that iterates all accounts. func List(params *stripe.AccountListParams) *Iter { return getC().List(params) } +// List returns an iterator that iterates all accounts. func (c Client) List(listParams *stripe.AccountListParams) *Iter { return &Iter{stripe.GetIter(listParams, func(p *stripe.Params, b *form.Values) ([]interface{}, stripe.ListMeta, error) { list := &stripe.AccountList{} @@ -102,15 +112,12 @@ func (c Client) List(listParams *stripe.AccountListParams) *Iter { })} } -// Iter is an iterator for lists of Accounts. -// The embedded Iter carries methods with it; -// see its documentation for details. +// Iter is an iterator for accounts. type Iter struct { *stripe.Iter } -// Account returns the most recent Account -// visited by a call to Next. +// Account returns the account which the iterator is currently pointing to. func (i *Iter) Account() *stripe.Account { return i.Current().(*stripe.Account) } diff --git a/charge/client.go b/charge/client.go index be5de87ae4..f64d983e65 100644 --- a/charge/client.go +++ b/charge/client.go @@ -1,4 +1,6 @@ -// Package charge provides the /charges APIs +// Package charge provides API functions related to charges. +// +// For more details, see: https://stripe.com/docs/api/go#charges. package charge import ( @@ -8,30 +10,30 @@ import ( "github.com/stripe/stripe-go/form" ) -// Client is used to invoke /charges APIs. +// Client is used to invoke APIs related to charges. type Client struct { B stripe.Backend Key string } -// New POSTs new charges. -// For more details see https://stripe.com/docs/api#create_charge. +// New creates a new charge. func New(params *stripe.ChargeParams) (*stripe.Charge, error) { return getC().New(params) } +// New creates a new charge. func (c Client) New(params *stripe.ChargeParams) (*stripe.Charge, error) { charge := &stripe.Charge{} err := c.B.Call(http.MethodPost, "/charges", c.Key, params, charge) return charge, err } -// Get returns the details of a charge. -// For more details see https://stripe.com/docs/api#retrieve_charge. +// Get retrieves a charge. func Get(id string, params *stripe.ChargeParams) (*stripe.Charge, error) { return getC().Get(id, params) } +// Get retrieves a charge. func (c Client) Get(id string, params *stripe.ChargeParams) (*stripe.Charge, error) { path := stripe.FormatURLPath("/charges/%s", id) charge := &stripe.Charge{} @@ -39,12 +41,12 @@ func (c Client) Get(id string, params *stripe.ChargeParams) (*stripe.Charge, err return charge, err } -// Update updates a charge's properties. -// For more details see https://stripe.com/docs/api#update_charge. +// Update updates a charge. func Update(id string, params *stripe.ChargeParams) (*stripe.Charge, error) { return getC().Update(id, params) } +// Update updates a charge. func (c Client) Update(id string, params *stripe.ChargeParams) (*stripe.Charge, error) { path := stripe.FormatURLPath("/charges/%s", id) charge := &stripe.Charge{} @@ -52,12 +54,12 @@ func (c Client) Update(id string, params *stripe.ChargeParams) (*stripe.Charge, return charge, err } -// Capture captures a charge not yet captured. -// For more details see https://stripe.com/docs/api#charge_capture. +// Capture captures a charge that's not yet captured. func Capture(id string, params *stripe.CaptureParams) (*stripe.Charge, error) { return getC().Capture(id, params) } +// Capture captures a charge that's not yet captured. func (c Client) Capture(id string, params *stripe.CaptureParams) (*stripe.Charge, error) { path := stripe.FormatURLPath("/charges/%s/capture", id) charge := &stripe.Charge{} @@ -65,12 +67,12 @@ func (c Client) Capture(id string, params *stripe.CaptureParams) (*stripe.Charge return charge, err } -// List returns a list of charges. -// For more details see https://stripe.com/docs/api#list_charges. +// List returns an iterator that iterates all charges. func List(params *stripe.ChargeListParams) *Iter { return getC().List(params) } +// List returns an iterator that iterates all charges. func (c Client) List(listParams *stripe.ChargeListParams) *Iter { return &Iter{stripe.GetIter(listParams, func(p *stripe.Params, b *form.Values) ([]interface{}, stripe.ListMeta, error) { list := &stripe.ChargeList{} @@ -85,15 +87,12 @@ func (c Client) List(listParams *stripe.ChargeListParams) *Iter { })} } -// Iter is an iterator for lists of Charges. -// The embedded Iter carries methods with it; -// see its documentation for details. +// Iter is an iterator for charges. type Iter struct { *stripe.Iter } -// Charge returns the most recent Charge -// visited by a call to Next. +// Charge returns the charge which the iterator is currently pointing to. func (i *Iter) Charge() *stripe.Charge { return i.Current().(*stripe.Charge) }