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

add feature to improve docs by having links to prs #662

Merged
merged 3 commits into from Oct 11, 2017

Conversation

Projects
None yet
3 participants
@mpacer
Copy link
Member

mpacer commented Sep 1, 2017

Per @Carreau's suggestion, I've added the ability to autolink to prs/issues by including a custom role.

I do not believe that this will work in notebooks, because there is no way to specify the custom role in markdown. But this still seems like an improvement for our changelogs (at the least).

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Sep 1, 2017

The only change from the ipython way of doing this is to not include the "PR" prefix because when multiple PRs are on the same lines this looks really bad & I don't want to try to batch the processing.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Sep 2, 2017

We made a package of our GH Sphinx extension so that we could use it across different repos without duplicating it. Is the thing about the 'PR' prefix important enough to make it worth duplicating the code again?

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Sep 13, 2017

didn't know about that… couldn't that be included with ghpro rather than as a separate package? They seem conceptually similar…

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Sep 13, 2017

Part of the reason I ask if we could move it over there is that that would also give us the opportunity to make this at least mildly more configurable and modular, so that it wouldn't be an issue to remove the PR prefix (which is inconsistent with how GitHub formats these links today and looks pretty strange when more than one PR are referenced on a single line).

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Sep 13, 2017

ghpro is a command line tool for doing operations on a Github repo. sphinxcontrib-github-alt is a Sphinx extension for referring to Github issues and PRs from Sphinx docs. It doesn't make any sense to me to join those into one package.

It's already under the Jupyter org (https://github.com/jupyter/sphinxcontrib_github_alt), so if we need some extra customisation, there's nothing stopping us doing that. I'm not convinced that the prefix thing is worth spending time on, but I'm happy to look over a pull request if you think it's worth doing.

@Carreau

This comment has been minimized.

Copy link
Contributor

Carreau commented Sep 13, 2017

It doesn't make any sense to me to join those into one package.

I agree with thomas on this one, the conceptual use case for GhPro and sphinxcontrib_github_alt is quite different. The only relevant part is that the magic :gh: role may need to contact GitHub to know if a number is an issue or a PR.

I would be happy to have the PR/issue prefix configurable. Second option, is as you said to make batch processsing, make :gh: take multiple parameters and render only the first with a prefix. For example :gh:`4,8,15,16,23,42` would render as PR #4, #8, #15, #16, #23 and #42 it should not be too hard.

@takluyver takluyver added the docs label Sep 15, 2017

@takluyver takluyver added this to the 5.4 milestone Oct 9, 2017

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 9, 2017

I've taken out the duplicate copy of the Sphinx github extension, and set it to use the packaged one instead. Any objections to merging this? We can make improvements to the extension as well, but I think having issues & PRs linked is already a nice step.

@takluyver takluyver merged commit 0a262a3 into jupyter:master Oct 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment