Skip to content

Add user best practices documentation #5604

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

Merged
merged 8 commits into from
Jun 24, 2022

Conversation

dstrain115
Copy link
Collaborator

  • Add document under 'start' about some basic best practices,
    like using 'cirq.X' instead of 'cirq.ops.X'.
  • Also added sections about immutability of gates, operations, and
    parameter resolvers.
  • Removed name of former product manager.

Fixes: #1373

- Add document under 'start' about some basic best practices,
like using 'cirq.X' instead of 'cirq.ops.X'.
- Also added sections about immutability of gates, operations, and
  parameter resolvers.
- Removed name of former product manager.
@dstrain115 dstrain115 requested review from a team, vtomole and cduck as code owners June 24, 2022 18:54
@dstrain115 dstrain115 requested a review from pavoljuhas June 24, 2022 18:54
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CirqBot CirqBot added the size: M 50< lines changed <250 label Jun 24, 2022
@dstrain115 dstrain115 requested a review from augustehirth June 24, 2022 18:59
@dstrain115
Copy link
Collaborator Author

FYI: test failure is a complex64 related flake

Copy link
Collaborator

@augustehirth augustehirth left a comment

Choose a reason for hiding this comment

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

LGTM, and a great addition. Thanks for writing this.

Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

Some nits

`cirq.ParamResolver` should not be modified after creation. If these objects
need to be modified, a new object should be created instead.

Violating this priniciple could cause problems in other parts of the code. For
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Violating this priniciple could cause problems in other parts of the code. For
Violating this principle could cause problems in other parts of the code. For

pavoljuhas and others added 4 commits June 24, 2022 13:43
@dstrain115
Copy link
Collaborator Author

@vtomole Hmmm... looks like there is some problem with your CLA that is now blocking the PR since I took your suggestions.

@pavoljuhas
Copy link
Collaborator

LGTM. There is also a best_practices.ipynb notebook,
but I suppose it is clear enough which best practices are which.

@vtomole
Copy link
Collaborator

vtomole commented Jun 24, 2022

Hmmm... looks like there is some problem with your CLA that is now blocking the PR since I took your suggestions.

Oh that's very new. Lemme look if i changed my default email.

Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

I don't know if CLAs expire, but I just resigned it. LGTM

@dstrain115 dstrain115 added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 24, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 24, 2022
@CirqBot CirqBot merged commit b5c628b into quantumlib:master Jun 24, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jun 24, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
- Add document under 'start' about some basic best practices,
like using 'cirq.X' instead of 'cirq.ops.X'.
- Also added sections about immutability of gates, operations, and
  parameter resolvers.
- Removed name of former product manager.

Fixes:  quantumlib#1373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "user best practices" to documentation
5 participants