-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat!: Move passes from algorithms
into a separate crate
#1100
Conversation
Otherwise extension inference fails in some tests.
Move one test into the hugr-passes crate, and refactor `depth()` into `utils`, to avoid code duplication.
algorithms
into a separat cratealgorithms
into a separate crate
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1100 +/- ##
==========================================
+ Coverage 86.81% 86.82% +0.01%
==========================================
Files 90 90
Lines 18501 18502 +1
Branches 18108 18109 +1
==========================================
+ Hits 16062 16065 +3
Misses 1600 1600
+ Partials 839 837 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have multiple comments about CI and metadata config, but it looks good.
@@ -140,7 +140,6 @@ | |||
// https://github.com/proptest-rs/proptest/issues/447 | |||
#![cfg_attr(test, allow(non_local_definitions))] | |||
|
|||
pub mod algorithm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we do #294 we should re-export algorithms here.
Normally we sub-crates are used for specific components of the library and the main library re-exports all non-internal things, but that will have to wait for now.
hugr-passes/Cargo.toml
Outdated
version = "0.1.0" | ||
edition = "2021" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version = "0.1.0" | |
edition = "2021" | |
version = "0.4.0" | |
edition = { workspace = true } | |
rust-version = { workspace = true } | |
license = { workspace = true } | |
readme = "README.md" | |
documentation = "https://docs.rs/hugr-passes/" | |
homepage = { workspace = true } | |
repository = { workspace = true } | |
description = "Compiler passes for Quantinuum's HUGR" | |
keywords = ["Quantum", "Quantinuum"] | |
categories = ["compilers"] |
Let's use the same major/minor version for all packages, to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case I can add version
under [workspace.package]
in the top-level Cargo.toml
and use {workspace = true }
in both packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to have to make a new release of hugr
whenever we do a new release of hugr-passes
? This shouldn't be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about having a matching minor number (0.x), even if the patch version varies.
But it may get desynchronised too, so I guess it's fine to leave it as 0.1.0.
The releases are done synchronously, but only for packages with changes.
See this PR for an example on how this looks.
Automated change generated by [`cargo-autoinherit`](https://github.com/mainmatter/cargo-autoinherit). This should simplify adding common dependencies on the new subcrates (#1100).
Ah, and two more tasks:
|
Co-authored-by: Agustín Borgna <[email protected]>
Just noticed that the top-level |
Would then need to update the link in the |
Yes! We should also replace the README symlink with a root README explaining the different crates, but that can be done later |
Btw, the token used by |
Moves all the code from `hugr` into a `hugr-core` subcrate. This way, `hugr-passes` (and any other new subcrate) can depend on the core definitions and get reexported in `hugr`. This PR does not. Change any definition or visibility in the code, as the renamed files are noisy enough. In particular, having `hugr-core` will let us unseal `HugrInternals` and `HugrMutInternals` so they can be used in `tket2`, but that's work for another PR. As a bonus, #1100 is no longer a breaking change since we can re-export the library without a cyclic dependency. Closes #294
## 🤖 New release * `hugr`: 0.4.0 -> 0.5.0 * `hugr-cli`: 0.1.0 * `hugr-core`: 0.1.0 * `hugr-passes`: 0.1.0 <details><summary><i><b>Changelog</b></i></summary><p> ## `hugr` <blockquote> ## 0.5.0 (2024-05-29) ### Bug Fixes - Missing re-exports in `hugr::hugr` ([#1127](#1127)) - Set initial version of hugr-core to 0.1.0 ([#1129](#1129)) ### Features - [**breaking**] Remove `PartialEq` impl for `ConstF64` ([#1079](#1079)) - [**breaking**] Allow "Row Variables" declared as List<Type> ([#804](#804)) - Hugr binary cli tool ([#1096](#1096)) - [**breaking**] Move passes from `algorithms` into a separate crate ([#1100](#1100)) - [**breaking**] Disallow nonlocal value edges into FuncDefn's ([#1061](#1061)) - [**breaking**] Move cli in to hugr-cli sub-crate ([#1107](#1107)) - Add verbosity, return `Hugr` from `run`. ([#1116](#1116)) - Unseal and make public the traits `HugrInternals` and `HugrMutInternals` ([#1122](#1122)) ### Refactor - [**breaking**] No Ports in TypeRow ([#1087](#1087)) - Add a `hugr-core` crate ([#1108](#1108)) </blockquote> ## `hugr-core` <blockquote> ## 0.1.0 (2024-05-29) ### Bug Fixes - Set initial version of hugr-core to 0.1.0 ([#1129](#1129)) ### Features - [**breaking**] Move cli in to hugr-cli sub-crate ([#1107](#1107)) - Make internals of int ops and the "int" CustomType more public. ([#1114](#1114)) - Unseal and make public the traits `HugrInternals` and `HugrMutInternals` ([#1122](#1122)) ### Refactor - Add a `hugr-core` crate ([#1108](#1108)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). --------- Co-authored-by: Agustin Borgna <[email protected]>
Closes #1000 .
Also:
hugr
crate into thehugr-passes
crate.utils
module and made it public.Hugr::new()
public. I thought I could avoid this by using theCFGBuilder
but doing so caused some extension-inference tests to fail, for reasons I couldn't fathom.Do we want to update the
release-plz
configuration? Not sure exactly what to do here.BREAKING CHANGE: The
algorithms
module is removed fromhugr
. It's functions are now in a separatehugr-passes
crate.