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

Feature/repo setup #1

Merged
merged 6 commits into from
Sep 17, 2024
Merged

Feature/repo setup #1

merged 6 commits into from
Sep 17, 2024

Conversation

jfriel-oqc
Copy link
Contributor

Setting up repo:

  • initial code
  • tests
  • formatting
  • repo setup, pipes etc

Note, no dependencies, pure python outside of dev dependencies

includes backwards compatibility and tests for this

@mmillmore
Copy link

Some generic comments on the structure;

  1. The src folder doesn't add anything. Guides like this https://docs.python-guide.org/writing/structure/ suggest not having it
  2. The test folder layout differs from our new standard testing structure at https://oxfordquantumcircuits.atlassian.net/wiki/spaces/QCAAS/pages/1614217217/Testing+Hierarchy
  3. There is a script for formatting using isort, black and autoflake. We have moved away from that to use ruff, which is much faster. Also, we configure that to run on save so no script is needed

tests/test_config.py Outdated Show resolved Hide resolved
tests/test_config.py Outdated Show resolved Hide resolved
@jfriel-oqc
Copy link
Contributor Author

Some generic comments on the structure;

  1. The src folder doesn't add anything. Guides like this https://docs.python-guide.org/writing/structure/ suggest not having it
  2. The test folder layout differs from our new standard testing structure at https://oxfordquantumcircuits.atlassian.net/wiki/spaces/QCAAS/pages/1614217217/Testing+Hierarchy
  3. There is a script for formatting using isort, black and autoflake. We have moved away from that to use ruff, which is much faster. Also, we configure that to run on save so no script is needed

agree with removing the src, done that.

This isn't a testing structure that's agreed upon across software and seems like overkill for a stand alone library that doesn't have multiple packages or need integration testing in the same way cloud does. So not making this change here

Equally for ruff, I have no opinion here, I'm going to leave this as is currently as it's the standard for the compiler team. But if they want it to be ruff or software team in general agrees to move everything over, then I'll change this no bother. Just not going to make that decision for another team

Copy link
Contributor

@bgsach bgsach left a comment

Choose a reason for hiding this comment

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

LGTM for a first pass

@bgsach bgsach merged commit baaebae into main Sep 17, 2024
18 checks passed
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.

3 participants