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

infra: update the pre-commit hook with linters #678

Merged
merged 18 commits into from
Sep 15, 2023
Merged

infra: update the pre-commit hook with linters #678

merged 18 commits into from
Sep 15, 2023

Conversation

AbeCoull
Copy link
Contributor

Issue #, if available:

Description of changes:

Testing done:

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (1ea24fb) 100.00% compared to head (170fbba) 100.00%.

❗ Current head 170fbba differs from pull request most recent head a06a217. Consider uploading reports for the commit a06a217 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #678   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          122       122           
  Lines         8144      8144           
  Branches      1826      1826           
=========================================
  Hits          8144      8144           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AbeCoull AbeCoull changed the title infra: update the pre-commit hook with linters and secrets check infra: update the pre-commit hook with linters Aug 18, 2023
@AbeCoull AbeCoull marked this pull request as ready for review August 18, 2023 19:27
@AbeCoull AbeCoull requested a review from a team as a code owner August 18, 2023 19:27
@AbeCoull AbeCoull requested a review from krneta August 18, 2023 19:27
Comment on lines -4 to -18
- id: tox
name: tox
stages: [push]
language: system
entry: tox
types: [python]
pass_filenames: false

- id: tox_integ_tests
name: tox integ tests
stages: [push]
language: system
entry: tox -e integ-tests
types: [python]
pass_filenames: false
Copy link
Member

Choose a reason for hiding this comment

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

Why remove these? We want the tests to pass before committing.

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 debated keeping the unit-tests run but I am hesitant around keeping the integ-tests. As these require an AWS account, I am not sure we should expect these to be ran by all contributors.

Copy link
Member

Choose a reason for hiding this comment

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

Removing integ tests makes sense, but I think unit tests are still good to have.

speller26
speller26 previously approved these changes Aug 28, 2023
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@@ -41,6 +41,17 @@ commands =
{[testenv:black]commands}
{[testenv:flake8]commands}

# Read only linter env
[testenv:linters_check]
Copy link
Member

Choose a reason for hiding this comment

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

Why not also run isort -c?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! I missed this as an option. I am also planning on using this test env here in our workflows: https://github.com/aws/amazon-braket-sdk-python/blob/main/.github/workflows/check-format.yml#L29. I shall add this check in for both.
Although, I have been eyeing up ruff a bit more recently to take on some of this work... out of scope for the PR.

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (687f706) 100.00% compared to head (c387d7b) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #678   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          122       122           
  Lines         8134      8134           
  Branches      1822      1822           
=========================================
  Hits          8134      8134           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AbeCoull AbeCoull requested a review from speller26 September 13, 2023 19:35
@AbeCoull AbeCoull requested a review from a team as a code owner September 13, 2023 19:35
@AbeCoull AbeCoull merged commit 5ca2d16 into main Sep 15, 2023
28 checks passed
@AbeCoull AbeCoull deleted the pre-commit branch September 15, 2023 13:06
virajvchaudhari added a commit that referenced this pull request Sep 26, 2023
* feat: add Aria2 enum (#653)

Co-authored-by: Viraj Chaudhari <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* infra: bump actions/checkout from 3.6.0 to 4.0.0 (#696)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 4.0.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@f43a0e5...3df4ab1)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* prepare release v1.55.0

* update development version to v1.55.1.dev0

* Revert "update: restricting parameter names to not collide with ones we use for OpenQASM generation. (#675)" (#701)

This reverts commit b158736.

* infra: update codeowner file to amazon-braket/braket-dev (#699)

Co-authored-by: Abe Coull <[email protected]>

* doc: Replace aws org with amazon-braket (#705)

* prepare release v1.55.1

* update development version to v1.55.2.dev0

* doc: change the sphinx requirement to be greater than 7.0.0 (#704)

Co-authored-by: Cody Wang <[email protected]>

* doc: add code contributors to the readme (#703)

Co-authored-by: Cody Wang <[email protected]>

* doc: Remove trailing backquotes (#706)

* infra: update the pre-commit hook with linters (#678)

* infra: update the pre-commit hook with linters and secrets check

Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* prepare release v1.55.1.post0

* update development version to v1.55.2.dev0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: ashlhans <[email protected]>
Co-authored-by: Cody Wang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ci <ci>
Co-authored-by: Milan <[email protected]>
Co-authored-by: Angela Guo <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
virajvchaudhari added a commit that referenced this pull request Sep 26, 2023
* feat: add queue position for tasks (#299)

* feat: add queue position for tasks

* update docstring for queue_position

* update docstrings

* update package_name

* update to enums and dataclass

* test: add integ test for task queue_position (#300)

* refactor: dataclass and file naming for queue info (#301)

* refactor: dataclass and file naming for queue info

* apply suggestions

* update: task queue position after refactor (#309)

* update: task queue position after refactor

* add queue_position type hint details, change order of info

* feat: queue position for hybrid jobs (#302)

* feat: queue position for hybrid jobs

* handle message return

* add docstring changes

* update docstring, and return None

* add context in dataclass

* update docstrings

* indent fix

* test: add integ test for jobs queue position (#310)

* feat: queue position for hybrid jobs

* handle message return

* add docstring changes

* update docstring, and return None

* add context in dataclass

* test: add integ test for jobs queue position

* remove comment

* minor fix

* remove unnecessary branching

* fix docstring merge

* remove dataclass redefinition after merge

* feat: queue depth for devices (#306)

* feat: queue depth for devices (dataclass version)

* add unit-test for queue depth

* modify docstrings, remove helper funcs

* add more info to docstrings

* minor test edit

* docstrings

* indent

* update QueuePriority to QueueType

* test: add integ test for queue_depth (#311)

* refactor: job and quantum_task keywords for queue_depth (#312)

* refactor: job and quantum_task keywords for queue_depth

* Update src/braket/aws/queue_information.py

Co-authored-by: Kshitij Chhabra <[email protected]>

---------

Co-authored-by: Kshitij Chhabra <[email protected]>

* deps: update boto3 version for queue visibility (#319)

* sync: public-main changes into feature/queue_visibility (#320)

* feat: add Aria2 enum (#653)

Co-authored-by: Viraj Chaudhari <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* infra: bump actions/checkout from 3.6.0 to 4.0.0 (#696)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 4.0.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@f43a0e5...3df4ab1)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* prepare release v1.55.0

* update development version to v1.55.1.dev0

* Revert "update: restricting parameter names to not collide with ones we use for OpenQASM generation. (#675)" (#701)

This reverts commit b158736.

* infra: update codeowner file to amazon-braket/braket-dev (#699)

Co-authored-by: Abe Coull <[email protected]>

* doc: Replace aws org with amazon-braket (#705)

* prepare release v1.55.1

* update development version to v1.55.2.dev0

* doc: change the sphinx requirement to be greater than 7.0.0 (#704)

Co-authored-by: Cody Wang <[email protected]>

* doc: add code contributors to the readme (#703)

Co-authored-by: Cody Wang <[email protected]>

* doc: Remove trailing backquotes (#706)

* infra: update the pre-commit hook with linters (#678)

* infra: update the pre-commit hook with linters and secrets check

Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* prepare release v1.55.1.post0

* update development version to v1.55.2.dev0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: ashlhans <[email protected]>
Co-authored-by: Cody Wang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ci <ci>
Co-authored-by: Milan <[email protected]>
Co-authored-by: Angela Guo <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Abe Coull <[email protected]>

* Revert "sync: public-main changes into feature/queue_visibility (#320)"

This reverts commit be6460c.

* update github script

* minor fix

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Kshitij Chhabra <[email protected]>
Co-authored-by: ashlhans <[email protected]>
Co-authored-by: Cody Wang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Milan <[email protected]>
Co-authored-by: Angela Guo <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants