updating scripts/genproto.sh by nate-double-u · Pull Request #13062 · etcd-io/etcd

nate-double-u · 2021-05-31T20:05:07Z

Changing API-reference generator

matching changes mentioned in API reference: typo in front matter and more website#330
generating page front-matter
reconsider where the output should go

fixes #13050
contributes to: #111, #12768

Files generated

Most recent commit:

Previous commits:

/cc @chalin

scripts/genproto.sh

codecov-commenter · 2021-05-31T20:53:21Z

Codecov Report

Merging #13062 (0266638) into main (c7cbc6b) will increase coverage by 2.66%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #13062      +/-   ##
==========================================
+ Coverage   63.85%   66.52%   +2.66%     
==========================================
  Files         430      425       -5     
  Lines       33142    33677     +535     
==========================================
+ Hits        21163    22402    +1239     
+ Misses       9942     9342     -600     
+ Partials     2037     1933     -104

Flag	Coverage Δ
all	`66.52% <ø> (+2.66%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
raft/quorum/quorum.go	`0.00% <0.00%> (-100.00%)`	⬇️
raft/tracker/state.go	`0.00% <0.00%> (-100.00%)`	⬇️
raft/util.go	`20.56% <0.00%> (-65.43%)`	⬇️
raft/confchange/restore.go	`39.53% <0.00%> (-55.82%)`	⬇️
raft/raftpb/confchange.go	`22.97% <0.00%> (-52.71%)`	⬇️
raft/quorum/voteresult_string.go	`0.00% <0.00%> (-50.00%)`	⬇️
raft/quorum/majority.go	`60.75% <0.00%> (-39.25%)`	⬇️
client/v3/internal/endpoint/endpoint.go	`46.34% <0.00%> (-39.03%)`	⬇️
raft/rawnode.go	`38.80% <0.00%> (-35.83%)`	⬇️
raft/tracker/progress.go	`62.50% <0.00%> (-34.73%)`	⬇️
... and 178 more

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 c7cbc6b...0266638. Read the comment docs.

nate-double-u · 2021-05-31T20:55:41Z

I don't think the test failures are related to the documentation updates.

scripts/genproto.sh

ptabor · 2021-06-01T15:25:04Z

I think that the tests fail due to shellcheck not understaning tripple quotes:

FAIL: (code:1):
scripts/genproto.sh:103:20: warning: Did you forget to close this single quoted string? [SC1078]
  % 'shellcheck' '-fgcc' 'build' 'test' 'scripts/build-release.sh' 'scripts/codecov_upload.sh' 'scripts/fix.sh' 'scripts/genproto.sh' 'scripts/install-marker.sh' 'scripts/release_mod.sh' 'scripts/test_lib.sh' 'scripts/update_dep.sh' 'scripts/updatebom.sh' './build.sh' './test.sh'
FAIL: 'run shellcheck -fgcc build test scripts/build-release.sh scripts/codecov_upload.sh scripts/fix.sh scripts/genproto.sh scripts/install-marker.sh scripts/release_mod.sh scripts/test_lib.sh scripts/update_dep.sh scripts/updatebom.sh ./build.sh ./test.sh' checking failed (!=0 return code)
scripts/genproto.sh:103:20: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
scripts/genproto.sh:108:1: note: This is actually an end quote, but due to next char it looks suspect. [SC1079]
FAIL: 'shellcheck' failed at Mon May 31 20:05:35 UTC 2021

scripts/genproto.sh:117:20: warning: Did you forget to close this single quoted string? [SC1078]
scripts/genproto.sh:117:20: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
scripts/genproto.sh:122:1: note: This is actually an end quote, but due to next char it looks suspect. [SC1079]
Error: Process completed with exit code 255.

chalin

See inline comments, and feedback by @ptabor.

@nate-double-u - have you been able to run this script? I'm getting sed errors under macOS. Maybe this currently only runs under Linux?

scripts/genproto.sh

nate-double-u · 2021-06-01T16:08:03Z

have you been able to run this script?

The files I linked in the description were generated. I admittedly did not run it on multiple platforms though.

nate-double-u · 2021-06-01T16:15:19Z

I'm getting sed errors under macOS. Maybe this currently only runs under Linux?

Doing a quick look around, it looks like there may be differences between sed on macOS and on Linux. I haven't tested the unmodified script on macOS, I'll have to test it to see if it has problems as well (or see if it's the additional line I added that's causing the issue)

nate-double-u · 2021-06-01T17:40:03Z

I think that the tests fail due to shellcheck not understaning tripple quotes

Thanks @ptabor, I'll take a closer look at that

nate-double-u · 2021-06-01T18:35:21Z

Files generated by the most recent commit:

nate-double-u · 2021-06-01T18:39:05Z

[edit] I had made a comment about tests, but as of the time of this writing the testing hasn't finished. I'm removing the comment and am waiting for the system to finish.

chalin

Hmm, you probably shouldn't be modifying go.mod and go.sum. (Yeah, I know that's what happens when we get the tools we need to run this script.)

I'm thinking that it makes more sense to have this script reside in the etcd website repo.

Thoughts @ptabor @spzala @gyuho?

nate-double-u · 2021-06-01T18:44:23Z

Hmm, you probably shouldn't be modifying go.mod and go.sum. (Yeah, I know that's what happens when we get the tools we need to run this script.)

Ya I just noticed that and have removed those edits.

nate-double-u · 2021-06-01T19:19:45Z

Rebased to clean up commits.

Latest force push updates .gitignore file, adds env vars, and adds new console output on success:

protodoc is finished.

To publish the API References, copy the following files
  - Documentation/dev-guide/api_reference_v3.md
  - Documentation/dev-guide/api_concurrency_reference_v3.md
to the etcd-io/website repo under the /content/en/docs/next/dev-guide/ folder.
(https://github.com/etcd-io/website/tree/main/content/en/docs/next/dev-guide)

./genproto SUCCESS

Generated files:

nate-double-u · 2021-06-01T20:33:03Z

I'm thinking that it makes more sense to have this script reside in the etcd website repo.

If we don't move it, do we want to consider building a GitHub action or something to build the references and push the files across to the etcd-io/website repo automatically? (I've found systems that automate this sort of thing to be fairly fragile in past, but does have the advantage of not requiring someone to remember to update the files manually).

[edit] we may want to consider something like this even if we do decide to move the script.

ptabor · 2021-06-02T06:54:21Z

scripts/genproto.sh

+  run sed -i -e 1,3d ${API_REFERENCE_CONCURRENCY_FILE}

  log_success "protodoc is finished."
+  log_success ""


Maybe:

log_warning "The files has NOT been automatically published on the website."

chalin · 2021-06-02T15:31:03Z

@ptabor - does this need to be ported to https://github.com/etcd-io/etcd/tree/release-3.5?

ptabor · 2021-06-02T15:34:55Z

@ptabor - does this need to be ported to https://github.com/etcd-io/etcd/tree/release-3.5?

Changing protos (so public wire formats) is rather unexpected during a minor version lifetime, so I don't think there are strong reasons to backport (but also costs are low).

nate-double-u · 2021-06-02T17:19:13Z

The last force push updates the output as per @ptabor's suggestion.

protodoc is finished.

The API References have NOT been automatically published to the website.

To publish the API References, copy the following files
  - Documentation/dev-guide/api_reference_v3.md
  - Documentation/dev-guide/api_concurrency_reference_v3.md
to the etcd-io/website repo under the /content/en/docs/next/dev-guide/ folder.
(https://github.com/etcd-io/website/tree/main/content/en/docs/next/dev-guide)

./genproto SUCCESS

Changing API-reference generator - matching changes mentioned in etcd-io/website#330 - generating page frontmater - updating .gitignore Signed-off-by: Nate W <4453979+nate-double-u@users.noreply.github.com>

nate-double-u · 2021-06-02T17:30:44Z

The last force-push is a copy edit:

protodoc is finished.

The API references have NOT been automatically published on the website.

To publish the API references, copy the following files
  - Documentation/dev-guide/api_reference_v3.md
  - Documentation/dev-guide/api_concurrency_reference_v3.md
to the etcd-io/website repo under the /content/en/docs/next/dev-guide/ folder.
(https://github.com/etcd-io/website/tree/main/content/en/docs/next/dev-guide)

./genproto SUCCESS

nate-double-u · 2021-06-02T18:41:18Z

Thanks for the feedback @ptabor & @chalin!

nate-double-u commented May 31, 2021

View reviewed changes

scripts/genproto.sh Outdated Show resolved Hide resolved

nate-double-u commented May 31, 2021

View reviewed changes

scripts/genproto.sh Outdated Show resolved Hide resolved

nate-double-u mentioned this pull request May 31, 2021

API reference: typo in front matter and more etcd-io/website#330

Closed

9 tasks

ptabor reviewed Jun 1, 2021

View reviewed changes

scripts/genproto.sh Outdated Show resolved Hide resolved

chalin reviewed Jun 1, 2021

View reviewed changes

scripts/genproto.sh Outdated Show resolved Hide resolved

chalin reviewed Jun 1, 2021

View reviewed changes

nate-double-u force-pushed the 13050-genproto-updates branch from f1785c3 to f223b8b Compare June 1, 2021 19:18

nate-double-u requested a review from ptabor June 1, 2021 19:21

ptabor reviewed Jun 2, 2021

View reviewed changes

ptabor approved these changes Jun 2, 2021

View reviewed changes

chalin approved these changes Jun 2, 2021

View reviewed changes

nate-double-u force-pushed the 13050-genproto-updates branch from f223b8b to 8f6e514 Compare June 2, 2021 17:18

updating scripts/genproto.sh

9e4dd4d

Changing API-reference generator - matching changes mentioned in etcd-io/website#330 - generating page frontmater - updating .gitignore Signed-off-by: Nate W <4453979+nate-double-u@users.noreply.github.com>

nate-double-u force-pushed the 13050-genproto-updates branch from 8f6e514 to 9e4dd4d Compare June 2, 2021 17:29

ptabor merged commit bd475d8 into etcd-io:main Jun 2, 2021

nate-double-u deleted the 13050-genproto-updates branch June 2, 2021 18:40

nate-double-u mentioned this pull request Jun 2, 2021

Ensure that generated documentation gets merged into website automatically #12768

Closed

nate-double-u mentioned this pull request Jun 16, 2021

Updating scripts/genproto.sh to refer to the correct folders on finish #13110

Closed

Conversation

nate-double-u commented May 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files generated

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented May 31, 2021

Codecov Report

Uh oh!

nate-double-u commented May 31, 2021

Uh oh!

Uh oh!

ptabor commented Jun 1, 2021

Uh oh!

chalin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nate-double-u commented Jun 1, 2021

Uh oh!

nate-double-u commented Jun 1, 2021

Uh oh!

nate-double-u commented Jun 1, 2021

Uh oh!

nate-double-u commented Jun 1, 2021

Uh oh!

nate-double-u commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chalin left a comment • 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!

nate-double-u commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-double-u commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-double-u commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ptabor Jun 2, 2021

Choose a reason for hiding this comment

Uh oh!

chalin commented Jun 2, 2021

Uh oh!

ptabor commented Jun 2, 2021

Uh oh!

nate-double-u commented Jun 2, 2021

Uh oh!

nate-double-u commented Jun 2, 2021

Uh oh!

nate-double-u commented Jun 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

nate-double-u commented May 31, 2021 •

edited

Loading

nate-double-u commented Jun 1, 2021 •

edited

Loading

chalin left a comment •

edited

Loading

nate-double-u commented Jun 1, 2021 •

edited

Loading

nate-double-u commented Jun 1, 2021 •

edited

Loading

nate-double-u commented Jun 1, 2021 •

edited

Loading