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

Support packageSrc in all versions of sbt #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mdedetrich
Copy link
Owner

I know that @raboof had reservations about doing such a change, but I wanted to see whether the deterministic issues carried over to sbtVersion (which I highly doubt).

@raboof Let me know if you still have reservations with this change, the idea is to merge this into main and manually test this against various pekko projects against a snapshot of sbt-apache-sonatype and if there is an issue then revert this commit.

@raboof
Copy link
Contributor

raboof commented Sep 24, 2024

I agree the sbtVersion is highly unlikely to cause determinism issues (like apache/pekko#1470), but I'm still concerned that adding more indirection here will make the determinism issues harder to diagnose.

Once we have hunted down and fixed apache/pekko#1470 this change is 👍 , but until that time I'm not sure we should apply it to pekko yet.

@mdedetrich
Copy link
Owner Author

Once we have hunted down and fixed apache/pekko#1470 this change is 👍 , but until that time I'm not sure we should apply it to pekko yet.

Yup this is no rush to merge, going to go on holidays for a few days without my laptop anyways. I will just quickly add a test with a different sbt version and we can merge when ready.

@mdedetrich mdedetrich force-pushed the support-packagesrc-all-versions-sbt branch from e5d5f94 to 36f96f1 Compare September 24, 2024 10:31
@mdedetrich
Copy link
Owner Author

@raboof I just added a regression test which I can confirm replicates and tests all of the expected behaviour. Once things have stabalized in Pekko with 1.10.2 we can look into merging this.

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