Feature: add code boilerplate check into circleci #813

SataQiu · 2019-08-14T09:05:44Z

Ⅰ. Describe what this PR did

Feature: add code format check into circleci

add code boilerplate check as a circleci job
make check-boilerplate.go always exit with 0, so we can catch the output
fix gofmt check bug (ignore vendor)

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

make boilerplate-check

Ⅴ. Special notes for reviews

allencloud · 2019-08-14T09:12:16Z

.circleci/config.yml

@@ -102,6 +112,7 @@ workflows:
  ci:
    jobs:
      - markdownlint-misspell-shellcheck
+      - gocode-format-check


You can add this in gocode-check-via-gometalinter-swagger.

allencloud · 2019-08-14T09:13:24Z

hack/boilerplate/check-boilerplate.go

@@ -116,14 +116,9 @@ func main() {
 		os.Exit(1)
 	}

-	hasErr := false
 	for _, filePath := range os.Args[1:] {
 		if err := verifyFile(filePath); err != nil {
 			fmt.Printf("error validating %q: %v\n", filePath, err)


This message should be printed in stderr, while now it is streamed to stdout, which seems to be incorrect. WDYT? @SataQiu

pouchcontainer/pouchlinter:v0.2.1 has no GCC installed.
Emm, I don't know where GCC is used...
Update pouchcontainer/pouchlinter:v0.2.1 or create a new job using circleci/golang:1.12.6 ?
WDYT? @allencloud

Latest pouchcontainer/pouchlinter:v0.2.2 has installed gcc if you need it.
ref: dragonflyoss/linter#17 (comment) 😄 hope it help for you.

OK! Let's bump to pouchcontainer/pouchlinter:v0.2.2. 😆

But in pouchcontainer/pouchlinter:v0.2.2 we removed gometaliner. We need to update circle-ci part to migrate to golangci-lint first. So I will open a PR tonight and after that you can work on this PR. Is it ok? I will finish it ASAP.

OK. Waiting for your good news. @yeya24

allencloud · 2019-08-20T01:57:09Z

hack/boilerplate/check-boilerplate.go

@@ -78,15 +77,15 @@ func checkBoilerplate(content string) error {
 		return fmt.Errorf("the file is missing a boilerplate")
 	}
 	if index < len(boilerplate) {
-		return errors.New("boilerplate has missing lines")
+		return fmt.Errorf("boilerplate has missing lines")


I am wondering if the change is unnecessary, since I think both are the same. WDYT?

Just want to clean something as I already imported fmt. ：）

allencloud · 2019-08-20T01:58:31Z

.circleci/config.yml

@@ -62,6 +62,9 @@ jobs:
          command: |
            make go-mod-vendor
            gometalinter --disable-all --skip vendor --skip test -E deadcode ./...
+      - run:


I think we need rebase this part onto the latest master branch.

codecov-io · 2019-08-20T05:19:40Z

Codecov Report

Merging #813 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #813      +/-   ##
==========================================
+ Coverage   39.63%   39.66%   +0.02%     
==========================================
  Files         109      109              
  Lines        6433     6429       -4     
==========================================
  Hits         2550     2550              
+ Misses       3675     3671       -4     
  Partials      208      208

Impacted Files	Coverage Δ
supernode/daemon/mgr/progress/progress_delete.go	`0% <ø> (ø)`	⬆️
hack/boilerplate/check-boilerplate.go	`36.58% <0%> (+3.25%)`	⬆️

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 017c312...c8c81cb. Read the comment docs.

SataQiu · 2019-08-20T05:26:09Z

@allencloud @yeya24
Considering that golanglint has checked gofmt, golint and govet, I recommend checking boilerplate separately.

yeya24 · 2019-08-20T05:32:08Z

@allencloud @yeya24
Considering that golanglint has checked gofmt, golint and govet, I recommend checking boilerplate separately.

Agree with that. If you want to only add boilerplate check, could please change the PR's title? 😄

SataQiu · 2019-08-20T05:38:01Z

Need review. @yeya24 @allencloud

yeya24 · 2019-08-20T06:03:49Z

hack/boilerplate-check.sh

+
+check() {
+    # boilerplate check
+    echo "CHECK: boilerpalte, check code boilerplate"


This sentence looks weird. Why not just use Begin to check code boilerplate here?

Because I write Begin to check code boilerplate in Makefile. Maybe just remove this? WDYT?

Yes, just keep one comment is OK.

yeya24 · 2019-08-20T06:04:29Z

hack/check.sh

@@ -10,7 +10,7 @@ cd "${curDir}/../" || return
 check() {
    # gofmt
    echo "CHECK: gofmt, check code formats"
-    result=$(find . -name '*.go' -print0 | xargs -0 gofmt -s -l -d 2>/dev/null)
+    result=$(find . ! -path './vendor*' -name '*.go' -print0 | xargs -0 gofmt -s -l -d 2>/dev/null)


Do you need to add vendor here? @SataQiu

This is an implicit bug. It does not exclude possible vendor directory. If we run make go-mod-vendor and then run make check, it will failed.

We should not check vendor, Right?

Yes, but after migrating to golangci-lint we don't run make go-mod-vendor now. So I think you can remove this.

Signed-off-by: SataQiu <[email protected]>

allencloud · 2019-08-20T17:36:09Z

Generally LGTM. While I still have a question, if I am new guy to contribute to dragonfly, when I forgot to add the boilerplate, then the CI would fail, but what exactly error message the CI output would throw to me? Is the error message friendly to a new guy?

SataQiu · 2019-08-21T03:07:57Z

Generally LGTM. While I still have a question, if I am new guy to contribute to dragonfly, when I forgot to add the boilerplate, then the CI would fail, but what exactly error message the CI output would throw to me? Is the error message friendly to a new guy?

Someting like this:

error validating "dfdaemon/config/config.go": the file is missing a boilerplate

If we have a command like make update-boilerplate, that will be great.

allencloud · 2019-08-21T06:01:41Z

If we have a command like make update-boilerplate, that will be great.

No, I do not think so. I think the boilerplate should be made from the developer manually. Everyone should be aware of the copyright part and some others. If the script helps to do this, then lots of developer can escape from deep understanding the duties and obligations.

allencloud · 2019-08-21T06:02:34Z

LGTM, thanks a lot for your great work.

…leci Feature: add code boilerplate check into circleci

pouchrobot added areas/test kind/feature size/S labels Aug 14, 2019

allencloud reviewed Aug 14, 2019

View reviewed changes

SataQiu force-pushed the add-codecheck-2-circleci branch 3 times, most recently from 35b601d to 54fc602 Compare August 14, 2019 10:24

pouchrobot added size/M and removed size/S labels Aug 14, 2019

SataQiu force-pushed the add-codecheck-2-circleci branch from 54fc602 to a39eae3 Compare August 14, 2019 10:29

pouchrobot added size/S and removed size/M labels Aug 14, 2019

weekly-digest bot mentioned this pull request Aug 18, 2019

Weekly Digest (11 August, 2019 - 18 August, 2019) #835

Closed

allencloud reviewed Aug 20, 2019

View reviewed changes

pouchrobot added the conflict/needs-rebase label Aug 20, 2019

allencloud reviewed Aug 20, 2019

View reviewed changes

SataQiu force-pushed the add-codecheck-2-circleci branch from a39eae3 to 09fa632 Compare August 20, 2019 05:16

pouchrobot added size/L and removed size/S labels Aug 20, 2019

SataQiu force-pushed the add-codecheck-2-circleci branch from 09fa632 to 8d274e4 Compare August 20, 2019 05:18

pouchrobot removed the conflict/needs-rebase label Aug 20, 2019

SataQiu force-pushed the add-codecheck-2-circleci branch from 8d274e4 to 348a128 Compare August 20, 2019 05:26

yeya24 removed the areas/test label Aug 20, 2019

SataQiu force-pushed the add-codecheck-2-circleci branch from d6afc5b to 348a128 Compare August 20, 2019 05:30

SataQiu changed the title ~~Feature: add code format check into circleci~~ Feature: add code boilerplate check into circleci Aug 20, 2019

pouchrobot added the areas/test label Aug 20, 2019

yeya24 reviewed Aug 20, 2019

View reviewed changes

SataQiu force-pushed the add-codecheck-2-circleci branch from a61e4a7 to fac9805 Compare August 20, 2019 15:16

add boilerplate-check into circleci

c8c81cb

Signed-off-by: SataQiu <[email protected]>

SataQiu force-pushed the add-codecheck-2-circleci branch from fac9805 to c8c81cb Compare August 20, 2019 15:18

allencloud self-assigned this Aug 20, 2019

pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Aug 21, 2019

allencloud merged commit 8e41c83 into dragonflyoss:master Aug 21, 2019

pouchrobot mentioned this pull request Aug 23, 2019

WeeklyReport of Dragonfly from 2019-08-16 to 2019-08-23 #856

Closed

starnop pushed a commit to starnop/Dragonfly that referenced this pull request Nov 27, 2019

Merge pull request dragonflyoss#813 from SataQiu/add-codecheck-2-circ…

5599920

…leci Feature: add code boilerplate check into circleci

inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019

Merge pull request dragonflyoss#813 from SataQiu/add-codecheck-2-circ…

7a8bc95

…leci Feature: add code boilerplate check into circleci

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: add code boilerplate check into circleci #813

Feature: add code boilerplate check into circleci #813

SataQiu commented Aug 14, 2019 •

edited

Loading

allencloud Aug 14, 2019

allencloud Aug 14, 2019

SataQiu Aug 14, 2019 •

edited

Loading

yeya24 Aug 14, 2019 •

edited

Loading

SataQiu Aug 14, 2019

yeya24 Aug 14, 2019

SataQiu Aug 14, 2019 •

edited

Loading

allencloud Aug 20, 2019

SataQiu Aug 20, 2019

allencloud Aug 20, 2019

codecov-io commented Aug 20, 2019 •

edited

Loading

SataQiu commented Aug 20, 2019

yeya24 commented Aug 20, 2019 •

edited

Loading

SataQiu commented Aug 20, 2019

yeya24 Aug 20, 2019

SataQiu Aug 20, 2019

yeya24 Aug 20, 2019

yeya24 Aug 20, 2019

SataQiu Aug 20, 2019

SataQiu Aug 20, 2019

yeya24 Aug 20, 2019 •

edited

Loading

allencloud commented Aug 20, 2019

SataQiu commented Aug 21, 2019 •

edited

Loading

allencloud commented Aug 21, 2019 •

edited

Loading

allencloud commented Aug 21, 2019 •

edited

Loading

Feature: add code boilerplate check into circleci #813

Feature: add code boilerplate check into circleci #813

Conversation

SataQiu commented Aug 14, 2019 • edited Loading

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SataQiu Aug 14, 2019 • edited Loading

Choose a reason for hiding this comment

yeya24 Aug 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SataQiu Aug 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Aug 20, 2019 • edited Loading

Codecov Report

SataQiu commented Aug 20, 2019

yeya24 commented Aug 20, 2019 • edited Loading

SataQiu commented Aug 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeya24 Aug 20, 2019 • edited Loading

Choose a reason for hiding this comment

allencloud commented Aug 20, 2019

SataQiu commented Aug 21, 2019 • edited Loading

allencloud commented Aug 21, 2019 • edited Loading

allencloud commented Aug 21, 2019 • edited Loading

SataQiu commented Aug 14, 2019 •

edited

Loading

SataQiu Aug 14, 2019 •

edited

Loading

yeya24 Aug 14, 2019 •

edited

Loading

SataQiu Aug 14, 2019 •

edited

Loading

codecov-io commented Aug 20, 2019 •

edited

Loading

yeya24 commented Aug 20, 2019 •

edited

Loading

yeya24 Aug 20, 2019 •

edited

Loading

SataQiu commented Aug 21, 2019 •

edited

Loading

allencloud commented Aug 21, 2019 •

edited

Loading

allencloud commented Aug 21, 2019 •

edited

Loading