Run check in CI for generated docs #783

lilic · 2019-06-07T18:21:08Z

What this PR does / why we need it:

The generated docs were out of date, so I reran the make generate. Also added a check for the generated docs, which will be now run as part of doc travis job.

The docs were out of date.

tariq1890 · 2019-06-07T18:22:20Z

docs/cli-arguments.md

      --logtostderr                                 log to standard error instead of files (default true)
      --metric-blacklist string                     Comma-separated list of metrics not to be enabled. The whitelist and blacklist are mutually exclusive.
      --metric-whitelist string                     Comma-separated list of metrics to be exposed. The whitelist and blacklist are mutually exclusive.
      --namespace string                            Comma-separated list of namespaces to be enabled. Defaults to ""
      --port int                                    Port to expose metrics on. (default 80)
      --skip_headers                                If true, avoid header prefixes in the log messages
-      --stderrthreshold severity                    logs at or above this threshold go to stderr
+      --skip_log_headers                            If true, avoid headers when openning log files


Suggested change

--skip_log_headers If true, avoid headers when openning log files

--skip_log_headers If true, avoid headers when opening log files.

Ah I see, it (make generate) was using the latest release, instead of master, let me fix this!

@tariq1890 Actually! I take that back, it's correct, it comes from klog, it's a typo there. If you do kube-state-metrics --help you will see it there :)

So if we want to use the make generate to update these docs, we need to keep it as is.

See kubernetes/klog#62 :)

tariq1890 · 2019-06-07T18:23:16Z

docs/cli-arguments.md

      --logtostderr                                 log to standard error instead of files (default true)
      --metric-blacklist string                     Comma-separated list of metrics not to be enabled. The whitelist and blacklist are mutually exclusive.
      --metric-whitelist string                     Comma-separated list of metrics to be exposed. The whitelist and blacklist are mutually exclusive.
      --namespace string                            Comma-separated list of namespaces to be enabled. Defaults to ""
      --port int                                    Port to expose metrics on. (default 80)
      --skip_headers                                If true, avoid header prefixes in the log messages
-      --stderrthreshold severity                    logs at or above this threshold go to stderr
+      --skip_log_headers                            If true, avoid headers when openning log files
+      --stderrthreshold severity                    logs at or above this threshold go to stderr (default 2)


Suggested change

--stderrthreshold severity logs at or above this threshold go to stderr (default 2)

--stderrthreshold severity logs at or above this threshold go to stderr (default 2).

tariq1890 · 2019-06-07T18:24:12Z

docs/horizontalpodautoscaler-metrics.md

I am not able to determine what this change exactly is.

Think it's a space, but these docs are generated so it's correct. :)

Cool! Thanks

tariq1890 · 2019-06-07T18:26:12Z

Makefile

@@ -122,7 +124,7 @@ generate: build-local embedmd
 	@$(GOPATH)/bin/embedmd -w `find . -path ./vendor -prune -o -name "*.md" -print`

 embedmd:
-	go get github.com/campoy/[email protected]
+	GO111MODULE=on go get github.com/campoy/[email protected]


I am not sure if this is desirable. This changes the go mod artifacts and will affect the go mod verify checks in the CI.

Yep you are right. But otherwise, go get fails.

I am not sure we want to add this @go mod tidy to the doccheck target. Can we try this with GO111MODULE=off instead? I don't see why this needs to be added to the go mod artifacts, assuming we are using embdedmd as a a CLI convenience in CI.

Interestingly enough with "off" hit this problem locally: cannot use path@version syntax in GOPATH mode

golang/go#29415

But I agree that was a hack, doing go mod tidy, I am sure there is a better way! :)

tariq1890 · 2019-06-07T19:11:12Z

Makefile

@@ -122,7 +124,7 @@ generate: build-local embedmd
 	@$(GOPATH)/bin/embedmd -w `find . -path ./vendor -prune -o -name "*.md" -print`

 embedmd:
-	go get github.com/campoy/embedmd@v1.0.0
+	GO111MODULE=off go get github.com/campoy/embedmd


Yup I was going to suggest this. This go mod limitation is very annoying. You can see what the prometheus team had to do in order to get around this issue:

https://github.com/prometheus/prometheus/blob/a5d39361ab0b53081fef5bdc6c722148716f175d/Makefile.common#L187-L201

^ That's just an FYI btw. I am definitely okay with go getting the embedmd without the versionedPath. Hopefully it works :)

That looks interesting 😄

I removed the version restrain, test pass now, tbh the embedmd project does not change much. But up to you?

Fine by me.

tariq1890 · 2019-06-07T19:15:09Z

Makefile

-doccheck:
+doccheck: generate
+	@echo "- Checking if the generated documentation is up to date..."
+	@git diff --exit-code --
 	@echo "- Checking if the documentation is up to date..."


Can we change this to something like Checking if the documentation is in sync with the code" ? Do you agree that this would perhaps add more clarity on what's going on ?

Makes sense, done ✔️

Whoops, I actually meant changing this "- Checking if the documentation is up to date..." to "-Checking if the documentation is in sync with the code"

The informational line that you had added in this PR was fine IMO.

ha, fixed and unfixed. Should be good now.

This way we will catch when we need to regenerate the docs, as the doccheck make target is run as part of CI.

tariq1890

/lgtm

Thanks @lilic for your contribution. Your patience is appreciated :).

k8s-ci-robot · 2019-06-07T19:37:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LiliC, tariq1890

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [tariq1890]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

CONTRIBUTING.md,docs/*: Run make generate

23b7f26

The docs were out of date.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 7, 2019

k8s-ci-robot requested review from andyxning and tariq1890 June 7, 2019 18:21

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 7, 2019

lilic force-pushed the fix-doc-args branch from ca1760e to 150b99b Compare June 7, 2019 18:24

tariq1890 reviewed Jun 7, 2019

View reviewed changes

lilic force-pushed the fix-doc-args branch 4 times, most recently from 7f915a7 to ee2efae Compare June 7, 2019 18:55

lilic changed the title ~~Fix doc args~~ Run check in CI for generated docs Jun 7, 2019

lilic force-pushed the fix-doc-args branch from ee2efae to 875ace1 Compare June 7, 2019 19:06

tariq1890 reviewed Jun 7, 2019

View reviewed changes

lilic force-pushed the fix-doc-args branch from 875ace1 to a21f1d1 Compare June 7, 2019 19:19

Makefile: Add check for generated docs

c38a2a1

This way we will catch when we need to regenerate the docs, as the doccheck make target is run as part of CI.

lilic force-pushed the fix-doc-args branch from a21f1d1 to c38a2a1 Compare June 7, 2019 19:33

tariq1890 approved these changes Jun 7, 2019

View reviewed changes

k8s-ci-robot assigned tariq1890 Jun 7, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2019

k8s-ci-robot merged commit 2626cb5 into kubernetes:master Jun 7, 2019

lilic deleted the fix-doc-args branch June 7, 2019 20:46

	--skip_log_headers If true, avoid headers when openning log files
	--skip_log_headers If true, avoid headers when opening log files.

	--stderrthreshold severity logs at or above this threshold go to stderr (default 2)
	--stderrthreshold severity logs at or above this threshold go to stderr (default 2).

Run check in CI for generated docs #783

Run check in CI for generated docs #783

Uh oh!

Conversation

lilic commented Jun 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilic Jun 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tariq1890 Jun 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tariq1890 left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jun 7, 2019

Uh oh!

Uh oh!

lilic Jun 7, 2019 •

edited

Loading

tariq1890 Jun 7, 2019 •

edited

Loading