Skip to content

Conversation

@jacopok
Copy link
Contributor

@jacopok jacopok commented Sep 30, 2025

Addresses #20.

It's quite simplistic, but I cannot see what else would be required. I can rebase after #21 is merged if that's preferred.

I added a test, probably being overzealous; I can remove it.

@jacopok jacopok changed the title Init Aspire object with existing flow ENH: Initialize Aspire object with existing flow Sep 30, 2025
@mj-will mj-will added the enhancement New feature or request label Oct 2, 2025
@mj-will mj-will self-requested a review October 2, 2025 07:53
Copy link
Owner

@mj-will mj-will left a comment

Choose a reason for hiding this comment

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

Thanks @jacopok, this looks good to me. I'm happy to get this in before #21

It seems the linting is failing, it's probably worth using the pre-commits to fix this since that will ensure the version used locally matches the CI.

@jacopok
Copy link
Contributor Author

jacopok commented Oct 2, 2025

I see, thanks! I will definitely set up the pre-commit hooks for the future; for the moment, I think the latest commit I made should be enough (I had manually linted the package source only, forgetting to do the same for the tests).

The test I added is now in the integration section of the CI - it may be worth moving to a new "unit test" section to be only run once in the future.

@mj-will
Copy link
Owner

mj-will commented Oct 2, 2025

Yeah, cleaning up the tests is on the list of things to do but for now I'm happy to keep this as is for now.

@mj-will mj-will merged commit a27c6dd into mj-will:main Oct 2, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants