Skip to content

Allow build.commands without build.tools #10281

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

Merged

Conversation

benjaoming
Copy link
Contributor

@benjaoming benjaoming commented Apr 30, 2023

  • update tests
  • update validate
  • check everything else that depends on some assumption

Fixes #9280

@benjaoming benjaoming changed the title Fix/build.os without build.tools Allow build.commands without build.tools Apr 30, 2023
@benjaoming
Copy link
Contributor Author

@benjaoming
Copy link
Contributor Author

benjaoming commented Apr 30, 2023

It works! But maybe we should add more documentation about the fact that the directory needs to be created?

https://docs.readthedocs.io/en/stable/build-customization.html#where-to-put-files

image

image

@benjaoming benjaoming marked this pull request as ready for review April 30, 2023 22:01
@benjaoming benjaoming requested a review from a team as a code owner April 30, 2023 22:01
@benjaoming benjaoming requested a review from humitos April 30, 2023 22:01
@benjaoming
Copy link
Contributor Author

@humitos I made a "positive" test (manually) to see that a build like this works. But is there something else to be aware of?

@humitos
Copy link
Member

humitos commented May 1, 2023

It works! But maybe we should add more documentation about the fact that the directory needs to be created?

I think adding just one sentence in the link you pasted mentioning this in the documentation is enough.

@humitos
Copy link
Member

humitos commented May 1, 2023

But is there something else to be aware of?

When working with the build process, I recommend you to always test locally a bunch of use-cases/branches from the test-builds repository -- not only the exact case you want to make it work. It can be tricky multiple times 😄

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.

Looks good, I think we are close to be able to merge it.

@benjaoming benjaoming requested a review from a team as a code owner May 1, 2023 19:21
@benjaoming benjaoming requested a review from ericholscher May 1, 2023 19:21
@benjaoming
Copy link
Contributor Author

I think adding just one sentence in the link you pasted mentioning this in the documentation is enough.

Added as a note box. I think it's great to add it in this PR :)

image

@benjaoming
Copy link
Contributor Author

But is there something else to be aware of?

When working with the build process, I recommend you to always test locally a bunch of use-cases/branches from the test-builds repository -- not only the exact case you want to make it work. It can be tricky multiple times 😄

I'll be trying out different branches that look relevant and get back 👍

@benjaoming
Copy link
Contributor Author

I did a bunch of builds searching for branch names containing "jobs" and "tools" and "commands" in test-builds and didn't find any positive negatives. The failing branches are failing later in the build.

image

@benjaoming benjaoming merged commit 8d896d1 into readthedocs:main May 2, 2023
@benjaoming benjaoming deleted the fix/build.os-without-build.tools branch May 2, 2023 10:20
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.

Config file: make build.tools not required when using build.commands
2 participants