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

Update extension based SP with content from main (#289) #290

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

SecondSkoll
Copy link
Collaborator

  • docs: Update customise.rst
  • chore: Sync all changes from main to extension branch
  • fix: fix build and doc check adds to gitignore
  • fix: workflows updates init script to provide dependency check
  • fix: removed file
  • fix: change to workflow sed
  • fix: change branch to support testing when staging PRs

* docs: Update customise.rst
* chore: Sync all changes from main to extension branch
* fix: fix build and doc check
  adds to gitignore
* fix: workflows
  updates init script to provide depenency check
* fix: removed file
* fix: change to workflow sed
* fix: change branch to support testing when staging PRs
@SecondSkoll
Copy link
Collaborator Author

Had to do some funky stuff to get this to work, as the tests require pulling from a branch in this project. So I essentially force merged into the extension-main-combined branch, so the init script clones from there, then submitted this PR from that branch.

When this is approved, the only change will be to line 53 in init.sh to change the branch back to the use-canonical-sphinx-extension branch. The tests will then fail - because it will be pulling the wrong content (but it will be the right content once it is merged).

This is the best we can do for the moment without a reimplementation of the init script.

@akcano
Copy link
Contributor

akcano commented Oct 1, 2024

@SecondSkoll on it, will do today or tomorrow.

Copy link
Contributor

@akcano akcano left a comment

Choose a reason for hiding this comment

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

LGTM with a few suggestions.

A couple of considerations:

  • We'll probably need to prune the Sphinx-specific and general dependencies and keep track of them down the road, if only because their number is growing, and I have a feeling we could do away with some.
  • A link to the documentation style guide should probably accompany markup-specific guides.

.gitignore Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
sp-docs/docs/build.rst Outdated Show resolved Hide resolved
sp-docs/docs/build.rst Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@SecondSkoll SecondSkoll left a comment

Choose a reason for hiding this comment

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

Some suggested adjustments in response to the review - so I can batch them.

.gitignore Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
sp-docs/docs/build.rst Outdated Show resolved Hide resolved
@akcano akcano merged commit fc7c712 into use-canonical-sphinx-extension Oct 3, 2024
6 checks passed
@SecondSkoll SecondSkoll deleted the extension-main-combined branch October 10, 2024 02:55
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.

2 participants