Skip to content

Support rebuilds, add pre-commit#19

Merged
Daltz333 merged 4 commits intosphinx-doc:masterfrom
chrisjsewell:rebuilds
Aug 30, 2020
Merged

Support rebuilds, add pre-commit#19
Daltz333 merged 4 commits intosphinx-doc:masterfrom
chrisjsewell:rebuilds

Conversation

@chrisjsewell
Copy link
Member

Fixes #18

@Daltz333
Copy link
Collaborator

@chrisjsewell I'm aiming to push a release tomorrow night around 10:30PM EST.

@Daltz333 Daltz333 requested a review from TheTripleV August 30, 2020 05:36
@chrisjsewell
Copy link
Member Author

I'm aiming to push a release tomorrow night around 10:30PM EST.

Cool cheers

If you don't mind I added a pre-commit config to the repo, and run the black CI via that (there's some other commented out checks you might consider adding)

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Aug 30, 2020

I'm also running the tests via tox.
I can add the tox.ini file if you want?
Basically both pre-commit and tox are super useful for not having to manually set up a development environment; you just run either and it will do it for you

Copy link
Collaborator

@TheTripleV TheTripleV left a comment

Choose a reason for hiding this comment

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

Could you add a dirhtml test case similar to the html test case you already added?

@TheTripleV
Copy link
Collaborator

Sure! I'm open to the tox file.

@chrisjsewell
Copy link
Member Author

Could you add a dirhtml test case

Done 👍

I figured out something a bit annoying; sphinx doesn't clean the build folder in-between each test.
Hence, the addition of the shutil.rmtree at the start of these rebuild tests

@TheTripleV
Copy link
Collaborator

Yeah I just noticed that while working on the other open pr. I'll make an issue as we should clean the build folder before every test.

@chrisjsewell
Copy link
Member Author

Yeah I just noticed that while working on the other open pr.

Yeh I guess just make a new fixture:

@pytest.fixture()
def app_clean(app):
    if Path(app.outdir).exists():
      shutil.rmtree(Path(app.outdir))
    yield app

(or clean after)

Copy link
Collaborator

@TheTripleV TheTripleV left a comment

Choose a reason for hiding this comment

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

The dirhtml test is using the html builder.

@chrisjsewell
Copy link
Member Author

The dirhtml test is using the html builder.

Ooops my bad 🤦

@TheTripleV
Copy link
Collaborator

LGTM. This will get merged in tomorrow.

@chrisjsewell
Copy link
Member Author

Brilliant, thanks for the great extension 😄

@Daltz333 Daltz333 changed the title Support rebuilds Support rebuilds, add pre-commit Aug 30, 2020
@Daltz333 Daltz333 merged commit 84aa2a3 into sphinx-doc:master Aug 30, 2020
TheTripleV added a commit to TheTripleV/sphinxext-rediraffe-1 that referenced this pull request Sep 30, 2020
* Support rebuilds

* Add pre-commit and CI

* Add dirhtml test

* Apply suggestions from code review

Co-authored-by: Vasista Vovveti <[email protected]>

Co-authored-by: Vasista Vovveti <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Properly support rebuilds

3 participants