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

Update pins in .scala-steward.conf #1589

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Update pins in .scala-steward.conf #1589

merged 1 commit into from
Dec 16, 2024

Conversation

pjfanning
Copy link
Contributor

@@ -11,6 +11,10 @@ updates.pin = [
{ groupId = "com.google.protobuf", artifactId = "protobuf-java", version = "3." }
# aeron 1.46 requires Java 17
{ groupId = "io.aeron", version = "1.45." }
# agrona 1.23 requires Java 17
{ groupId = "org.agrona", artifactId = "agrona" version = "1.22." }
Copy link
Member

Choose a reason for hiding this comment

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

I seem to remember the reason we dodn't let scala steward update agrona is because aeron and agrona are somewhat tightly coupled, so we only update agrona when we update aeron (to the exact version that aeron expects).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the problem is that with scala steward, we might accept the aeron update and forget the agrona one - with this change, we will be warned about a new jar - we don't need to merge it but we can research if the change is a good idea

Copy link
Member

Choose a reason for hiding this comment

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

The comments around the versions in project/Dependencies.scala seem fairly clear - but I see at https://github.com/real-logic/aeron/blob/1.45.1/build.gradle#L59 it might be less tightly coupled than I thought, so 👍 I suppose

Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

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

Lgtm

@pjfanning pjfanning merged commit 784c596 into main Dec 16, 2024
9 checks passed
@pjfanning pjfanning deleted the pjfanning-patch-1 branch December 16, 2024 23:50
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