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

Revert "Build: pass PATH environment variable to Docker container (#10133)" #10206

Closed

Conversation

ericholscher
Copy link
Member

@ericholscher ericholscher commented Mar 28, 2023

This reverts #10172 & #10133

@ericholscher ericholscher requested a review from a team as a code owner March 28, 2023 19:24
@ericholscher ericholscher requested a review from humitos March 28, 2023 19:24
@ericholscher ericholscher force-pushed the revert-bc4bceda2a55497ee07a5a470f1a0d54200c6873 branch from 8d3ed2e to 7b93e58 Compare March 28, 2023 19:26
@ericholscher ericholscher added the PR: hotfix Pull request applied as hotfix to release label Mar 28, 2023
Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

Looks like we need to run pre-commit

@stsewd
Copy link
Member

stsewd commented Mar 29, 2023

We may want to run pre-commit in another commit, and do a merge commit instead of squash+rebase, to avoid any conflicts when merging main into our release branches.

@ericholscher
Copy link
Member Author

I'm not really sure how to make this pass, it passes locally:

-> pre-commit run --files readthedocs/doc_builder/environments.py
check for added large files..............................................Passed
check python ast.........................................................Passed
check for case conflicts.................................................Passed
check that executables have shebangs.................(no files to check)Skipped
check json...........................................(no files to check)Skipped
check that scripts with shebangs are executable..........................Passed
check toml...........................................(no files to check)Skipped
check vcs permalinks.....................................................Passed
debug statements (python)................................................Passed
fix utf-8 byte order marker..............................................Passed
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
trim trailing whitespace.................................................Passed
mixed line ending........................................................Passed
autoflake................................................................Passed
prospector...............................................................Passed
darker...................................................................Passed
blacken-docs.............................................................Passed

@stsewd
Copy link
Member

stsewd commented Mar 30, 2023

@ericholscher that's because the commit is already done, you can run pre-commit run --from-ref main --to-ref HEAD

@ericholscher
Copy link
Member Author

This was merged into main manually 👍

@stsewd stsewd deleted the revert-bc4bceda2a55497ee07a5a470f1a0d54200c6873 branch March 30, 2023 21:21
humitos added a commit that referenced this pull request May 22, 2023
This is another attempt to fix this issue. It follows the suggestion posted by
@bensze01 in #10103. It
looks simple and I think it makes sense to give it a try since it doesn't seem
to be harmful.

I tested it locally with the branch
https://github.com/readthedocs/test-builds/tree/build-commands-multiline and it
worked fine. Note that without this PR the branch linked fails.

Related #10133
Related #10119
Related #10206

External PRs that should be solved by this one:
* pypi/warehouse#12953
* pypi/warehouse#12953

Closes #10103
Closes #10208
humitos added a commit that referenced this pull request May 23, 2023
* Build: allow multi-line commands on `build.commands`

This is another attempt to fix this issue. It follows the suggestion posted by
@bensze01 in #10103. It
looks simple and I think it makes sense to give it a try since it doesn't seem
to be harmful.

I tested it locally with the branch
https://github.com/readthedocs/test-builds/tree/build-commands-multiline and it
worked fine. Note that without this PR the branch linked fails.

Related #10133
Related #10119
Related #10206

External PRs that should be solved by this one:
* pypi/warehouse#12953
* pypi/warehouse#12953

Closes #10103
Closes #10208

* Build: use `;` to separate PREFIX from COMMAND
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: hotfix Pull request applied as hotfix to release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants