Skip to content
This repository has been archived by the owner on Mar 30, 2018. It is now read-only.

Add make checks to PR template. Resolves #1905 #1906

Conversation

srderson
Copy link
Contributor

Description

An updated pull request template that discusses make checks

Motivation and Context

Fixes #1905
This change was discussed with @ghaskins and @rameshthoomu. Please review this proposal. I have remove go vet and goimports from the checklist and instead discuss how make checks will be run in CI and how it can be run locally.

How Has This Been Tested?

No new tests required and this is only a change to the PR template.

Checklist:

  • I have added a Signed-off-by
  • Either no new documentation is required by this change, OR I added new documentation
  • Either no new tests are required by this change, OR I added new tests
  • I have run goimports, go vet, and golint. I have cleaned up all valid errors and warnings in code I have added or modified. These tools may generate false positives. Don't be worried about ignoring some errors or warnings. The goal is clean, consistent, and readable code.

Signed-off-by: Sheehan Anderson sheehan@us.ibm.com

@dco-bot
Copy link

dco-bot commented Jun 17, 2016

Hi srderson,

Thanks for submitting this pull request!

I can confirm that the DCO1.1 sign-off has been included. It is okay to process this pull request.

dco-bot

<!-- To uncheck box, add a space: [ ] -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [] I have added a [Signed-off-by](https://github.com/hyperledger/fabric/blob/master/CONTRIBUTING.md#legal-stuff).
- [] I have added documentation to cover my changes or this change requires no new documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check: does the following suggestion make this clearer or worse (to a native English speaker) ?
I have either added documentation to cover my changes or this change requires no new documentation.

Otherwise, sorry in advance for my broken English ;-)

@christo4ferris
Copy link
Contributor

I like the phrasing that @JonathanLevi suggested @srderson otherwise LGTM

@srderson
Copy link
Contributor Author

@JonathanLevi @christo4ferris Thanks for the feedback. I have updated the PR template with the suggested language.

@christo4ferris
Copy link
Contributor

LGTM thanks!

@ghaskins
Copy link
Contributor

LGTM

@ghaskins ghaskins merged commit 6905fec into hyperledger-archives:master Jun 21, 2016
@srderson srderson mentioned this pull request Jun 23, 2016
4 tasks
lhaskins pushed a commit to lhaskins/fabric-archive that referenced this pull request Oct 14, 2016
…5/add_make_checks_to_pr_template

Add make checks to PR template. Resolves hyperledger-archives#1905
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Update pull request template to include make checks
6 participants