Skip to content

Conversation

@IanButterworth
Copy link
Contributor

IanButterworth added a commit to IanButterworth/julia that referenced this pull request Oct 25, 2025
@mortenpi
Copy link
Member

I'm a bit confused by the changes / fix.

  1. We now pass the checkdocs_ignored_modules when we gather the bindings.. which is probably fine, but it should be redundant, since doc.blueprint.modules should already be filtered?
  2. I don't really understand why the sorting of keys would make a difference?

@IanButterworth
Copy link
Contributor Author

  1. Julia base docs only partially cover Pkg. Pkg has its own docs. So we need to pass Pkg for docs but exclude from the docs check

  2. I don't either really. That part was Claude thinking random ordering was the reason for the random results

@mortenpi
Copy link
Member

  1. Julia base docs only partially cover Pkg. Pkg has its own docs. So we need to pass Pkg for docs but exclude from the docs check

But they should already be excluded by here, no?

blueprint = DocumentBlueprint(
Dict{String, Page}(),
submodules(modules; ignore = Set(checkdocs_ignored_modules)),
)

@IanButterworth
Copy link
Contributor Author

@mortenpi I don't know. It might be worth investigating this with JuliaLang/julia#55267 without my guesses. This PR was necessary for me to get Pkg excluded

Copy link
Collaborator

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

This probably should get a changelog entry?

I still don't quite understand why the sorting is needed, and that bothers me a bit; but in the end, I think we should be pragmatic: if both sort! are demonstrably necessary to solve a concrete problem in Julia, we probably should merge it anyway.

Perhaps with some code comments added in that explain why the sorting is done (as in: linking to this PR resp. to the underlying GitHub issue)?

@IanButterworth IanButterworth marked this pull request as draft October 30, 2025 22:17
@IanButterworth
Copy link
Contributor Author

To be clearer, I don't know if these are the correct fixes, and I'm asking for help from devs more familiar with Documenter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants