Skip to content

extensions: Use bazel build for generating docs rst#16749

Merged
phlax merged 2 commits intoenvoyproxy:mainfrom
phlax:extensions-rstgen
Jun 2, 2021
Merged

extensions: Use bazel build for generating docs rst#16749
phlax merged 2 commits intoenvoyproxy:mainfrom
phlax:extensions-rstgen

Conversation

@phlax
Copy link
Copy Markdown
Member

@phlax phlax commented Jun 1, 2021

Commit Message: extensions: Use bazel build for generating docs rst
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #16749 was opened by phlax.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jun 1, 2021
@phlax phlax force-pushed the extensions-rstgen branch 3 times, most recently from d5cc718 to e2b097c Compare June 1, 2021 13:18
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax force-pushed the extensions-rstgen branch from e2b097c to 4c318ab Compare June 1, 2021 13:35
@phlax phlax changed the title [WIP] extensions: Use bazel build for generating docs rst extensions: Use bazel build for generating docs rst Jun 1, 2021
@phlax phlax requested a review from mattklein123 June 1, 2021 14:01
@phlax phlax marked this pull request as ready for review June 1, 2021 14:01
@phlax phlax requested a review from moderation June 1, 2021 14:01
Signed-off-by: Ryan Northey <ryan@synca.io>
Comment on lines +39 to +43
pip_install(
name = "docs_pip3",
requirements = "@envoy//tools/docs:requirements.txt",
extra_pip_args = ["--require-hashes"],
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What are your thoughts on capturing metadata for the Python deps? I added this data as comments with an example at https://github.com/envoyproxy/envoy/blob/main/bazel/repositories_extra.bzl#L26-L37. Some issues:

  • some new deps don't have this data
  • no automated checkers
  • not machine readable
  • duplicitive

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep - mostly the issues - ie hard to maintain and in some cases it just duplicates what is there in the egg

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thinking maybe we remove the existing comment metadata until we think we need a better solution and agree on a way to do it? (doesn't have to be in this PR which looks like it is on its way to passing CI)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure - im happy to follow that up after

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice

@moderation
Copy link
Copy Markdown
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Jun 1, 2021
@phlax phlax merged commit 1a05155 into envoyproxy:main Jun 2, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
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.

3 participants