Skip to content
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

INFRA: Drop numeric XX- from markdown file names #1332

Closed
wants to merge 11 commits into from

Conversation

effigies
Copy link
Collaborator

Closes #1323.

@effigies effigies added infrastructure exclude-from-changelog This item will not feature in the automatically generated changelog labels Oct 20, 2022
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

argh, I introduced merge conflicts by merging some PRs (I think). Sorry. +1 on merging this as soon as conflicts are resolved.

A potential re-ordering of sections can then follow in another PR that won't be as prone to conflicts.

@sappelhoff sappelhoff requested a review from CPernet as a code owner October 23, 2022 09:38
@sappelhoff
Copy link
Member

  • note that although the CI "checkmark" for your last commit 3427934 is green, there were failures that I tried to fix with the following commits
  • however, the build_docs_pdf check is stubborn, and I can't see what's wrong: locally the PDF builds well ... when logged into circleci via ssh, the problem is the same (perhaps obviously)

@sappelhoff
Copy link
Member

sappelhoff commented Oct 23, 2022

Ah, I see one consequence of dropping the XX prefixes now. The PDF build gets screwed up in its order:

# Get all input files
markdown_list = []
for root, dirs, files in os.walk("."):
for file in files:
fpath = os.path.join(root, file)
if fpath.endswith(".md") and fpath not in EXCLUDE:
markdown_list.append(fpath)
elif fpath.endswith("index.md"):
# Special role for index.md
index_page = fpath

# Add input files to command
# The filenames in `markdown_list` will ensure correct order when sorted
cmd += [str(root / index_page)]
cmd += [str(root / i) for i in ["../../metadata.yml"] + sorted(markdown_list)]

☝️ as can be seen in these snippets, the build was relying on the order being determined via the file names.

but that doesn't explain why the CI fails (while locally, everything works, albeit with the screwed up ordering)

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

PDF build is broken (see comments above)

@effigies effigies marked this pull request as draft October 26, 2022 15:07
@effigies
Copy link
Collaborator Author

Seems this needs more thought.

@sappelhoff
Copy link
Member

sappelhoff commented Oct 27, 2022

probably we could parse mkdocs.yml's "Nav":

... then we wouldn't need to have the loop that's dependent on file names, and we wouldn't have to maintain a duplicate list of files just for the PDF build

I attempted this in #1343 ... perhaps merging that and pulling it into this branch would fix the build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop numeric XX- from markdown file names
2 participants