Skip to content

chore(fix): Installing mkdocs related decencies from docs.yml #402

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

chore(fix): Installing mkdocs related decencies from docs.yml #402

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3247530
chore: installing mkdocs-print-site-plugin and testing validate-docs-…
siddharth-khatsuriya May 14, 2025
096ff6c
chore: installing dependency from docs.yml
siddharth-khatsuriya May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion .github/workflows/reusable-build-test-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,39 @@ jobs:
submodules: false
persist-credentials: false
- name: Installing requirements
shell: bash
run: |
pip install pip -U
pip install mkdocs==1.6.1 mkdocs-material==9.6.9 poetry
pip install poetry
DOCS_FILE=""
if [[ -f ".github/workflows/docs.yml" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having to parse the docs.yml workflow file to get the dependencies to install, it make more sense to keep those dependencies in the pyproject.toml file under a specific group (i.e. docs) and then use directly from there to install these dependencies (using poetry install --only docs).
WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree.

Copy link
Member

Choose a reason for hiding this comment

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

to give a bit of context on why it was done this way - so that doc dependencies can be upgraded separately and not to introduce new dependencies for TA teams

those dependencies would need to be added manually for every repository, we can't roll those out because it will mess poetry.lock and devs would need to update it manually which is quite a procedure

Copy link
Contributor

@mkolasinski-splunk mkolasinski-splunk May 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with argument about updating poetry.lock. On the other hand I don't really like current approach where versions are hardcoded in docs.yml workflow:

  1. complex libs retrieval for validation - as in this PR
  2. if certain repo requires different libraries, template rollout will need to handle exceptions.

What about introducing requirements-docs.txt for now and encouraging devs to introduce new group to pyproject.toml as Darshan mentioned?

DOCS_FILE=".github/workflows/docs.yml"
elif [[ -f ".github/workflows/docs.yaml" ]]; then
DOCS_FILE=".github/workflows/docs.yaml"
fi
echo "docs yaml file found at: $DOCS_FILE"
if [[ -f "$DOCS_FILE" ]]; then
PIP_COMMAND=$(awk '{
for (i = 1; i <= NF; i++) {
if ($i ~ /^pip[0-9.]*$/ && $(i+1) == "install") {
for (j = i+2; j <= NF; j++) {
printf "%s ", $j
}
}
}
}' $DOCS_FILE)
if [ -z "$PIP_COMMAND" ]; then
echo "No pip install command found in $DOCS_FILE!"
else
echo "Installing dependencies: $PIP_COMMAND"
# shellcheck disable=SC2086
pip install $PIP_COMMAND
fi
else
echo "$DOCS_FILE file not found. No dependencies installed for mkdocs"
echo "Installing default pip install mkdocs==1.6.1 mkdocs-material==9.6.9"
pip install mkdocs==1.6.1 mkdocs-material==9.6.9
fi
- name: validate
id: validate
run: |
Expand Down
Loading