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

serrors: Add nogo check #3213

Merged
merged 4 commits into from
Oct 3, 2019
Merged

Conversation

oncilla
Copy link
Contributor

@oncilla oncilla commented Oct 3, 2019

This changes adds a nogo rule that ensures the varargs input to
serrors.{New, WithCtx, Wrap, WrapStr} is well-formed.

fixes #3196


This change is Reviewable

This changes adds a linter that ensures the varargs input to
serrors.{New, WithCtx, Wrap, WrapStr} is well-formed.
@oncilla oncilla added the feature New feature or request label Oct 3, 2019
@oncilla oncilla added this to the Q4S1 milestone Oct 3, 2019
@oncilla oncilla requested a review from sustrik October 3, 2019 06:53
Copy link
Contributor

@sustrik sustrik left a comment

Choose a reason for hiding this comment

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

Remove 'changes' from the description of the PR.

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @oncilla)


WORKSPACE, line 293 at r1 (raw file):

    name = "com_github_oncilla_gochecks",
    commit = "d6344eb0440fcd1b62d95b9e0eb64f1350638420",
    importpath = "github.com/oncilla/gochecks",

Add at least a one-line readme to the GitHub repo so that it's obvious what it is for.

@karampok
Copy link
Contributor

karampok commented Oct 3, 2019


WORKSPACE, line 293 at r1 (raw file):

Previously, sustrik (Martin Sustrik) wrote…

Add at least a one-line readme to the GitHub repo so that it's obvious what it is for.

I would suggest also that this goes into anapaya/scion space in case someone would like to make a change.

@oncilla
Copy link
Contributor Author

oncilla commented Oct 3, 2019

WORKSPACE, line 293 at r1 (raw file):

Previously, sustrik (Martin Sustrik) wrote…

Add at least a one-line readme to the GitHub repo so that it's obvious what it is for.

I would suggest also that this goes into anapaya/scion space in case someone would like to make a change.

As this is a personal project I'm doing outside of work, I don't think this should be in scionproto.

@karampok
Copy link
Contributor

karampok commented Oct 3, 2019


WORKSPACE, line 293 at r1 (raw file):

Previously, Oncilla wrote…

As this is a personal project I'm doing outside of work, I don't think this should be in scionproto.

ack

Copy link
Contributor

@sustrik sustrik left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sustrik)

Copy link
Contributor

@sustrik sustrik left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@oncilla oncilla changed the title serrors: Add linter serrors: Add nogo check Oct 3, 2019
@oncilla oncilla merged commit 456109b into scionproto:master Oct 3, 2019
@oncilla oncilla deleted the pub-serrors-lint branch October 3, 2019 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

serrors: Write linter
3 participants