-
Notifications
You must be signed in to change notification settings - Fork 40
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
Include the module's docstring in the header #100
Conversation
8b62af1
to
8b8a421
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable. However, the PR would need to update the golden files for the tests - otherwise, it's causing every e2e stardoc test to fail.
This is currently not being rendered anywhere in the output, despite being parsed into the stardoc_output.proto. Fixes bazelbuild#25
8b8a421
to
e831911
Compare
setup.bzl
Outdated
urls = ["https://github.com/bazelbuild/bazel-skylib/archive/16de038c484145363340eeaf0e97a0c9889a931b.tar.gz"], # 2020-08-11 | ||
sha256 = "96e0cd3f731f0caef9e9919aa119ecc6dace36b149c2f47e40aa50587790402b", | ||
strip_prefix = "bazel-skylib-16de038c484145363340eeaf0e97a0c9889a931b", | ||
sha256 = "58f558d04a936cade1d4744d12661317e51f6a21e3dd7c50b96dc14f3fa3b87d", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note, this update is needed to print a better failure message on diff_test (there hasn't been a release there for over a year :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/bazelbuild/bazel-skylib/releases/tag/1.1.1 released this week :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test is, unfortunately, failing on Windows: https://buildkite.com/bazel/stardoc/builds/231#054cdb1e-52a4-45cc-8a62-463ccf788d76
Would you mind bumping the skylib dep to 1.1.1 (which has some additional Windows-specific fixes) to see if that allows the test to pass?
And if that doesn't help, we'd have to go back to the old test runner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still fails, I should file a bug on skylib's diff_test I guess
okay @tetromino done |
test/BUILD
Outdated
|
||
licenses(["notice"]) # Apache 2.0 | ||
|
||
sh_test( | ||
diff_test( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for simplifying the code!
It has a bug on Windows
@tetromino green now with reduced scope |
Thanks. Root-caused the Windows problem to #110 |
This is currently not being rendered anywhere in the output, despite being parsed into the stardoc_output.proto.
It seems like an omission