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.

Sign up for GitHub

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

Jump to bottom

Combined contribute_to_qiskit.rst with contributing.md #11217

Merged
merged 5 commits into from
Nov 20, 2023
Merged

Combined contribute_to_qiskit.rst with contributing.md #11217

merged 5 commits into from
Nov 20, 2023

Conversation

javabster
Copy link
Contributor

Summary

This PR copies over relevant information in contribute_to_qiskit.rst into the CONTRIBUTING.md, meaning we can safely remove https://qiskit.org/documentation/contributing_to_qiskit.html when the time comes.

This PR also updates CONTRIBUTING.md to no longer use the term "terra"

Details and comments

@javabster javabster added the documentation Something is not clear or an error documentation label Nov 8, 2023
@javabster javabster requested a review from a team as a code owner November 8, 2023 21:40
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Nov 8, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 6909379076

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 535 unchanged lines in 52 files lost coverage.
  • Overall coverage decreased (-1.0%) to 85.91%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/sabre_layout.rs 1 98.31%
crates/accelerate/src/sabre_swap/mod.rs 1 97.35%
crates/accelerate/src/stochastic_swap.rs 1 87.96%
crates/qasm2/src/error.rs 1 97.56%
qiskit/circuit/library/blueprintcircuit.py 1 92.24%
qiskit/circuit/library/evolved_operator_ansatz.py 1 85.12%
qiskit/circuit/library/generalized_gates/unitary.py 1 92.11%
qiskit/opflow/state_fns/operator_state_fn.py 1 80.22%
qiskit/opflow/state_fns/state_fn.py 1 81.88%
qiskit/primitives/backend_sampler.py 1 98.84%
Totals Coverage Status
Change from base Build 6801884624: -1.0%
Covered Lines: 65949
Relevant Lines: 76765
💛 - Coveralls

@Eric-Arellano Eric-Arellano changed the title Combined contribut_to_qiskit.rst with contributing.md Combined contribute_to_qiskit.rst with contributing.md Nov 9, 2023
Eric-Arellano
Eric-Arellano previously approved these changes Nov 9, 2023
View reviewed changes
Copy link
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks, Abby!

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md
pip install -e .
```

## Installing Qiskit from source
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to instead link out to the new Install From Source page in OneDocs? I think the main motivation would be de-duplication.

I'm fine with it all being in this guide, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy with that, but maybe the maintainers have a different opinion? this would be the new link if we decided to change: https://docs.quantum-computing.ibm.com/start/install-qiskit-source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The content is a bit different, so maybe theres reason to keep both?

@javabster @Eric-Arellano
Update CONTRIBUTING.md
d393fd4 
Co-authored-by: Eric Arellano <[email protected]>
@Eric-Arellano Eric-Arellano mentioned this pull request Nov 9, 2023
Closed
@Eric-Arellano
Copy link
Collaborator

Bump @Qiskit/terra-core

jakelishman
jakelishman reviewed Nov 15, 2023
View reviewed changes
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.

Please can we also make sure that the now-obsolete file is deleted from the repository as part of this PR?

CONTRIBUTING.md Outdated
Comment on lines 1 to 8
# Contributing

First read the overall project contributing guidelines. These are all
included in the qiskit documentation:
Qiskit is an open-source project committed to bringing quantum computing to
people of all backgrounds. This page describes how you can join the Qiskit
community in this goal.

https://qiskit.org/documentation/contributing_to_qiskit.html

## Contributing to Qiskit Terra

In addition to the general guidelines there are specific details for
contributing to terra, these are documented below.

### Contents
Copy link
Member

Choose a reason for hiding this comment

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

The refactoring here seems to have broken the logical heading levels a little bit. Not sure what heading structure you prefer, but at the moment we go from level 1 to level 3 then back to level 2, which isn't great for accessibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah good point, the heading levels where a bit janky to beign with so I tried to tidy them up a bit, but must have missed this one

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
jakelishman
jakelishman approved these changes Nov 17, 2023
View reviewed changes
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.

Thanks Abby, I'm fine with this. Given Eric's concerns about maybe needing to make an emergency docs deployment, perhaps the best strategy is to leave this PR approved but not merged until we're ready on the qiskit.org side (or wherever it's getting deployed)? Having this PR with the old-file removal means git will force us to resolve the change if anybody happens to make a change to the old form before we actually merge this, so we can't accidentally lose changes.

@jakelishman
Copy link
Member

Apologies - I misremembered: they were your concerns, not Eric's.

@jakelishman jakelishman mentioned this pull request Nov 17, 2023
Merged
@javabster
Copy link
Contributor Author

It's only 1 week until we get the redirects going and I highly doubt we'll need to do any docs deployments at this later stage, so I'm happy to take that risk and deploy this now :)

@javabster javabster added this pull request to the merge queue Nov 20, 2023
Merged via the queue into Qiskit:main with commit b3a422a Nov 20, 2023
FabianBrings pushed a commit to FabianBrings/qiskit that referenced this pull request Nov 27, 2023
@javabster @Eric-Arellano
@jakelishman @FabianBrings
Combined contribute_to_qiskit.rst with contributing.md (Qiskit#11217)
951a9a7 
* Combined contribut_to_qiskit.rst with contributing.md

* Update CONTRIBUTING.md

Co-authored-by: Eric Arellano <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jake Lishman <[email protected]>

* applied review comments and removed old contributing guide

* fixed docs warning

---------

Co-authored-by: Eric Arellano <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakelishman jakelishman jakelishman approved these changes

@Eric-Arellano Eric-Arellano Eric-Arellano left review comments

Assignees
No one assigned
Labels
documentation Something is not clear or an error documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@javabster @qiskit-bot @coveralls @Eric-Arellano @jakelishman