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

Refer to IBM docs #10640

Merged
merged 5 commits into from
Aug 21, 2023
Merged

Refer to IBM docs #10640

merged 5 commits into from
Aug 21, 2023

Conversation

Eric-Arellano
Copy link
Collaborator

@Eric-Arellano Eric-Arellano commented Aug 15, 2023

Closes #10594.

Screenshot 2023-08-21 at 7 30 42 AM

@coveralls
Copy link

coveralls commented Aug 15, 2023

Pull Request Test Coverage Report for Build 5903102057

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.04%) to 87.282%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 2 91.14%
Totals Coverage Status
Change from base Build 5902014500: 0.04%
Covered Lines: 74373
Relevant Lines: 85210

💛 - Coveralls

docs/index.rst Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
@Eric-Arellano Eric-Arellano changed the title [blocked] Refer to IBM docs Refer to IBM docs Aug 18, 2023
@Eric-Arellano Eric-Arellano marked this pull request as ready for review August 18, 2023 13:16
@Eric-Arellano Eric-Arellano requested a review from a team as a code owner August 18, 2023 13:16
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

A bit minor, but as I understand it, much of this is directly lifted from Abby's PR Qiskit/qiskit-metapackage#1791 - it might be nice to make sure Abby's credited at least as a co-author on the PR.

Comment on lines +132 to +134
"theme_announcement": "🎉 Qiskit is getting a new documentation experience on IBM Quantum!",
"announcement_url": "https://docs.quantum-computing.ibm.com/",
"announcement_url_text": "Check it out",
Copy link
Member

Choose a reason for hiding this comment

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

This feels like a slightly weird coupling between the theme and the repository, but not a huge deal.

Minor: the split of "namespacing" one variable with theme_ but not the others is a bit odd to me.

(Personally, I'd prefer we namespace everything Qiskit-specific with a qiskit_ prefix, since we have to share the namespace with everything Sphinx-related, and reading the conf file, it's unclear to me now what's related to the theme and what's not, but that's a relatively minor issue and out of scope any way.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I agree. Too late to change now that it's been released though.

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

As far as I'm concerned this is fine, but probably best to get a review from someone properly on the docs squad / involved in the 1XP discussions before merge.

@Eric-Arellano
Copy link
Collaborator Author

As far as I'm concerned this is fine, but probably best to get a review from someone properly on the docs squad / involved in the 1XP discussions before merge.

We've internally shared screenshots with Jules and she approved :)

@Eric-Arellano Eric-Arellano added documentation Something is not clear or an error documentation Changelog: None Do not include in changelog labels Aug 21, 2023
@Eric-Arellano
Copy link
Collaborator Author

Ah, @jakelishman @mtreinish I forgot to put stable-backport-potential when opening this. It would help the 1XP team a lot if this went out on stable docs, not only the dev docs.

@Eric-Arellano Eric-Arellano added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Aug 21, 2023
@Eric-Arellano Eric-Arellano added this to the 0.25.2 milestone Aug 21, 2023
@jakelishman
Copy link
Member

Either way, best have Abby sign off on the PR to keep the full spirit of the review process, since she'd suggested changes.

@jakelishman
Copy link
Member

And yeah, this is inherently stable for backport, so no trouble adding the label.

Copy link
Contributor

@javabster javabster left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@jakelishman jakelishman added this pull request to the merge queue Aug 21, 2023
Merged via the queue into Qiskit:main with commit 94820e6 Aug 21, 2023
12 of 13 checks passed
@Eric-Arellano Eric-Arellano deleted the use-banner branch August 21, 2023 19:46
mergify bot pushed a commit that referenced this pull request Aug 21, 2023
* Refer to IBM docs

* Review feedback

* Use live links

* Properly attribute code to Abby!

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>

---------

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
(cherry picked from commit 94820e6)
github-merge-queue bot pushed a commit that referenced this pull request Aug 22, 2023
* Refer to IBM docs

* Review feedback

* Use live links

* Properly attribute code to Abby!

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>

---------

Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
(cherry picked from commit 94820e6)

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog documentation Something is not clear or an error documentation stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
Archived in project
Status: Done
Development

Successfully merging this pull request may close these issues.

Add 1xp docs announcement section to documentation home page
5 participants