-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Convert deprecation policy and maintainer guide to markdown #11218
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 6935485297
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Abby!
Co-authored-by: Eric Arellano <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generally seems fine to me in principle, although I do think that it's a regression to move from hosting this in a properly rendered documentation build GitHub markdown views.
Should this PR also remove the old versions of these files?
There are other open PRs that are modifying the related files here, such as #11205, which have logical conflicts.
DEPRECATION.md
Outdated
- The alternative path must be in place for one minor version before any | ||
warnings are issued. For example, if we want to replace the function `foo()` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In all the bullets in this list, the continuation-line indentation looks like it's got an extra space in it. Not that it affects the display, it just makes it a bit harder to update.
DEPRECATION.md
Outdated
*migration guide* might be needed. Please write these guides in Qiskit's documentation at | ||
https://github.com/Qiskit/documentation/tree/main/docs/api/migration-guides. Once | ||
the migration guide is written and published, deprecation | ||
messages and documentation should link to it (use the `additional_msg: str` argument for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style: I don't think we need to include the type hint for an argument when mentioning it in prose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this a few months ago. I think it's useful for people to know what the argument is referring to. I agree it's not "necessary", but I think it's useful for only 5 extra characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd have thought the API documentation is a more appropriate place for this. To me it adds noise and makes it harder to scan (especially because this is the second time it happens).
MAINTAINING.md
Outdated
Refer to https://qiskit.github.io/qiskit_sphinx_theme/apidocs/index.html for how to create and | ||
write effective API documentation, such as setting up the RST files and docstrings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still going to be a relevant link after the full 1XP migration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This site will still exist for the sake of the Qiskit Ecosystem. Its guidance on API docs should still be relevant for Qiskit itself.
Not yet. We're going to set up redirects from the qiskit.org pages to these markdown files as part of Qiskit/documentation#216 (still WIP!) |
I think this PR should still remove the files from this repository; |
I realize that will only impact |
Co-authored-by: Jake Lishman <[email protected]>
The only reason I'd see for not removing the files yet in this PR is in case we need to do any emergency qiskit.org docs deployments between now and the redirects kicking in on 29th Nov. If we're fine risking that then I'll remove the files, I don't have a strong opinion either way. At some point (out of scope for this PR) we should also remove the old tutorials and how-to guides etc as well, they've all been either migrated or superseded by 1docs content. We can check the commit history to see if anything was added since the migration or 0.45 release that isn't in 1docs |
|
||
contributing_to_qiskit | ||
deprecation_policy | ||
maintainers_guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to delete this file and contributing_to_qiskit.rst too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah jake asked me to remove it in the other contributing guide PR as well so just thought i'd do it in both to avoid merge conflicts whichever gets merged first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this PR will want to delete the maintainers guide and deprecation policy, but leave contributing guide. You'll get a merge conflict with your PR, but such it is 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes, Abby. Similar to #11217, if you're worried about not being ready for this change on the backend yet, shall we leave this approved but unmerged til you're ready?
Having the old files removed in the PR means we can't accidentally lose any changes from other PRs to the old file, because git will force us to resolve a merge conflict, so we fail safe in that case.
(btw, I know there's a docs-build failure just because of the contributing_to_qiskit.rst
file being orphaned at the moment - it's just an approval in spirit, rather than knowing we're immediately going to merge, since I assume #11218 will go first.)
This PR replaces any qiskit.org links with 1XP or github equivalents. Need to wait to merge until I can update the deprecation policy and maintainer guide links after Qiskit/qiskit#11218 merges. This should partially resolve #297 , however for the api links we'll need to make updates for those directly in the repos where the API refs live (maybe this is beyond the scope of the original issue?) --------- Co-authored-by: Eric Arellano <[email protected]>
…1218) * Converted deprecation policy and maintainer guide to markdown * Apply suggestions from code review Co-authored-by: Eric Arellano <[email protected]> * Added more context around writing docs * Update DEPRECATION.md Co-authored-by: Jake Lishman <[email protected]> * fixed some formatting issues * Removed deprecation_policy.rst and maintainers_guide.rst * Apply suggestions from code review Co-authored-by: Jake Lishman <[email protected]> * update index.rst --------- Co-authored-by: Eric Arellano <[email protected]> Co-authored-by: Jake Lishman <[email protected]>
This PR replaces any qiskit.org links with 1XP or github equivalents. Need to wait to merge until I can update the deprecation policy and maintainer guide links after Qiskit/qiskit#11218 merges. This should partially resolve Qiskit#297 , however for the api links we'll need to make updates for those directly in the repos where the API refs live (maybe this is beyond the scope of the original issue?) --------- Co-authored-by: Eric Arellano <[email protected]>
Summary
This PR creates markdown versions of the deprecation policy and maintainer guide, so we can safely remove https://qiskit.org/documentation/deprecation_policy.html and https://qiskit.org/documentation/maintainers_guide.html
Details and comments