-
Notifications
You must be signed in to change notification settings - Fork 21
Add sphinx_module support #366
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
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: Click to expand output |
10334d0 to
e028ba1
Compare
|
The created documentation from the pull request is available at: docu-html |
f2d8490 to
5af3382
Compare
718388b to
37594b3
Compare
37594b3 to
f6a6196
Compare
|
@ramceb can you use rules_python instead of score_tooling here? https://rules-python.readthedocs.io/en/latest/api/sphinxdocs/sphinxdocs/sphinx_docs_library.html |
f6a6196 to
91f5186
Compare
91f5186 to
83a72b9
Compare
| $(SRCS) | ||
| """, | ||
| tools = ["@score_docs_as_code//scripts_bazel:generate_sourcelinks"], | ||
| visibility = ["//visibility:public"], |
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.
AFAIK this must be public. I also dont understand the comment in the next line. Looks like a merge problem?
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.
fixed
| load("@rules_pkg//pkg:mappings.bzl", "pkg_files", "strip_prefix") | ||
| load("@rules_python//sphinxdocs:sphinx.bzl", "sphinx_build_binary", "sphinx_docs") | ||
| load("@score_tooling//:defs.bzl", "score_virtualenv") | ||
| load("@score_tooling//bazel/rules/rules_score:rules_score.bzl", "sphinx_module") |
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.
What is the reason of bringing this in?
Trying to understand why we need/should use this?
Will this be compatible with every consumer / downstream?
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.
It is not breaking anything up to now. So yes it is compatible. And in the process group there was a discussion that we will start a poc for persistency:
https://github.com/orgs/eclipse-score/discussions/407#discussioncomment-15605348
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.
To my understanding this is POC development performed by BMW, which we can basically merge as long as it doesn't break the productive docs build. Currently it lacks features in comparison to the current build setup, so it's too early to even discuss which is "better".
We need to be careful to not confuse docs users or management expectations.
7b9394e to
10d63bb
Compare
|
discussed and agreed to merge |
📌 Description
🚨 Impact Analysis
✅ Checklist