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

Document new build jobs #11900

Merged
merged 5 commits into from
Jan 13, 2025
Merged

Document new build jobs #11900

merged 5 commits into from
Jan 13, 2025

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jan 9, 2025

  • Document all new jobs
  • Replace almost all usage of build.commands with build.jobs
  • Unify examples, as they are all now part of build.jobs

Ref https://github.com/readthedocs/readthedocs-ops/issues/1582


📚 Documentation previews 📚

@stsewd stsewd marked this pull request as ready for review January 9, 2025 23:02
@stsewd stsewd requested a review from a team as a code owner January 9, 2025 23:02
@stsewd stsewd requested a review from agjohnson January 9, 2025 23:02
@stsewd stsewd requested a review from ericholscher January 9, 2025 23:02
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

This is great! 🎉

docs/user/build-customization.rst Outdated Show resolved Hide resolved
docs/user/build-customization.rst Outdated Show resolved Hide resolved
docs/user/config-file/v2.rst Outdated Show resolved Hide resolved
Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Thanks for writing the docs for this! 🎉

Comment on lines 133 to 134
build.commands
~~~~~~~~~~~~~~
Copy link
Member

Choose a reason for hiding this comment

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

I would call this "Completely override the build process" as it is presented at the beginning of the page. Also, I'd use ---- for the section:

Suggested change
build.commands
~~~~~~~~~~~~~~
Completely override the build process
-----------------------------------------------------------

Copy link
Member

Choose a reason for hiding this comment

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

I would avoid having a title with just build.commands since it doesn't communicate too much the users.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this should under this section, but renamed it to "alternative syntax"


Extend the build process
------------------------
Extend or override the build process
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Extend or override the build process
Extend or partially override the build process

Copy link
Member Author

Choose a reason for hiding this comment

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

build.jobs allows for both, partial and complete override, the only difference with build.commands is the way it's done and the support for apt packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

I liked how the items above linked to the relevant sections previously.

I don't think we need to get into specifics of partially/full overriding in the section headings. In fact, I'd be clearer with both of these items/headings:

  • Customize our standard build process
  • Define a build process from scratch

That is, describe the effect to the end user more than technically describe what's happening in the build.

docs/user/config-file/v2.rst Show resolved Hide resolved
@stsewd stsewd merged commit 46dee70 into main Jan 13, 2025
8 checks passed
@stsewd stsewd deleted the document-new-build-jobs branch January 13, 2025 18:16
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

This looks great, I'm excited to ship these docs out 🚀


Extend the build process
------------------------
Extend or override the build process
Copy link
Contributor

Choose a reason for hiding this comment

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

I liked how the items above linked to the relevant sections previously.

I don't think we need to get into specifics of partially/full overriding in the section headings. In fact, I'd be clearer with both of these items/headings:

  • Customize our standard build process
  • Define a build process from scratch

That is, describe the effect to the end user more than technically describe what's happening in the build.

Comment on lines +20 to +21
the ``create_environment``, ``install``, and ``build`` jobs will use the default commands for the selected tool,
otherwise, they will default to run nothing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't do suggest for some reason 🤷

I'd make this two sentences.

...jobs will use the default commands for the selected tool.
If no tool configuration is specified, these jobs won't execute anything by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

probably because the PR was already merged :)

Comment on lines +125 to +126
You can access the search from the Read the Docs :term:`dashboard`,
or by using the :doc:`/server-side-search/api`.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are referring to the search view UI in the dashboard, that feels unrelated to documentation search and build configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

this section was already in the document, but it refers to search support for all tools.

- uv pip install -r requirements.txt
build:
html:
- uv run sphinx-build -T -b html docs $READTHEDOCS_OUTPUT/html
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice improvement 👍

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.

4 participants