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

feat: Added "make lint" target and added to "make test" target #302

Merged
merged 2 commits into from
Jan 10, 2022

Conversation

marcpfuller
Copy link
Contributor

@marcpfuller marcpfuller commented Jan 5, 2022

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-bootstrap/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?) N/A
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?) Did an integration test details below
  • I have opened a PR for the related docs change (if not, why?) N/A

Testing Instructions

Ran edgex-go using local go-mod-bootstrap in both security and no-secty to make sure everything came up and no errors in logs

New Dependency Instructions (If applicable)

N/A

Signed-off-by: Marc-Philippe Fuller <[email protected]>
@marcpfuller marcpfuller changed the title Added "make lint" target and added to "make test" target - Issue #280 feat: Added "make lint" target and added to "make test" target Jan 5, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 5, 2022

Codecov Report

Merging #302 (ec8edae) into main (9fdf17b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #302   +/-   ##
=======================================
  Coverage   53.67%   53.67%           
=======================================
  Files          17       17           
  Lines        1008     1008           
=======================================
  Hits          541      541           
  Misses        446      446           
  Partials       21       21           
Impacted Files Coverage Δ
bootstrap/secret/secure.go 77.46% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fdf17b...ec8edae. Read the comment docs.

@lenny-goodell lenny-goodell requested a review from bnevis-i January 6, 2022 15:18
Makefile Outdated Show resolved Hide resolved
bootstrap/bootstrap.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@bnevis-i bnevis-i left a comment

Choose a reason for hiding this comment

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

LGTM.

One optional change that I've made to the other Makefiles is to break out "go test" into a "unittest" target.

That way, the unit tests (the most important thing) run first, then the linter, then vet, gofmt, etc.

Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

@lenny-goodell lenny-goodell merged commit d813076 into edgexfoundry:main Jan 10, 2022
@bnevis-i bnevis-i linked an issue Jan 11, 2022 that may be closed by this pull request
jinlinGuan pushed a commit to IOTechSystems/go-mod-bootstrap that referenced this pull request Apr 29, 2022
…foundry#302)

* feat: Added golangci-lint to makefile

Signed-off-by: Marc-Philippe Fuller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "make lint" target and add to "make test" target
4 participants