Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@cgranade
Copy link
Contributor

@cgranade cgranade commented Jan 8, 2020

This PR uses a new experimental linter to begin identifying and resolving code quality issues throughout the libraries, including:

  • Missing or incomplete documentation
  • Incorrect case conventions in operation and function names
  • Unsupported markup in documentation summaries

This PR also includes manual fixes to whitespacing, shorthand use for adjoint and controlled declarations, and other similar code quality issues.
Finally, this PR also includes the suggestions made by @alan-geller in #198, and other miscellaneous improvements made while resolving the issues above.

auxiliary: 'T,
system: 'S)
: Unit {
body (...){
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a space after the body's argument tuple.

Copy link
Contributor

@ScottCarda-MS ScottCarda-MS left a comment

Choose a reason for hiding this comment

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

Please double-check the consistency of tabs and spaces in all changed files. I stopped marking them with individual comments.

@cgranade
Copy link
Contributor Author

Thanks for all the feedback, @ScottCarda-MS and @alan-geller, I really appreciate all your comments — especially on a draft PR! I'll work on continuing to address them and on finishing the PR.

Chris Granade and others added 13 commits January 13, 2020 17:57
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
Co-Authored-By: Alan Geller <alan.geller@microsoft.com>
@cgranade
Copy link
Contributor Author

There's always more to be done, but I think this is a pretty good step in the right direction, so I'm marking as ready for review now.

@cgranade cgranade marked this pull request as ready for review January 14, 2020 02:29
Copy link
Member

@anpaz anpaz left a comment

Choose a reason for hiding this comment

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

Looks great overall.
There are a couple of breaking changes (mostly on things that should be private). I would recommend running and end-to-end QDK build before merging to master.

@cgranade
Copy link
Contributor Author

Good suggestion, @anpaz-msft. It looks like the Quantum-NC repo has an accidental dependence on private APIs, such that the end-to-end build failed. I'll go on and address that in the NC repo before merging this in.

cgranade pushed a commit to microsoft/QuantumKatas that referenced this pull request Jan 15, 2020
* Use qualified opens to adapt to new location for MeasureAllZ.

* Undo qualified open in reference implementation.

* Revert to open/as.

* Fix minor mistake w/ prev commit.
@cgranade cgranade merged commit 4bec1d5 into master Jan 15, 2020
@cgranade cgranade deleted the cgranade/code-quality branch January 15, 2020 20:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants