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

Fix #974 – Scala 3 macro cannot find Writes for Seq[Map[String, T]] #993

Conversation

sgodbillon
Copy link
Contributor

Moved the deprecated method DefaultWrites.mapWrites into LowPriorityWrites to get rid of the ambiguity.

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you squashed your commits?
  • Have you added copyright headers to new files?
  • Have you updated the documentation?
  • Have you added tests for any changed functionality?

Fixes

Fixes #974 – Scala 3 macro cannot find Writes for Seq[Map[String, T]]

Purpose

What does this PR do?

Background Context

Why did you take this approach?

References

Are there any relevant issues / PRs / mailing lists discussions?

@sgodbillon
Copy link
Contributor Author

About binary compatibility – there is another approach we can take that would be binary compatible: keep mapWrites where it is but drop the implicit keyword. That breaks source compatibility, but only when it is imported explicitly so the impact would be minimal. Let me know what you think!

@cchantep
Copy link
Member

cchantep commented Mar 4, 2024

@sgodbillon sgodbillon force-pushed the sgodbillon/974-seq-of-map-aambiguous-implicits-scala3 branch from a662ff1 to 1107a14 Compare March 4, 2024 21:58
@sgodbillon
Copy link
Contributor Author

Seems it needs to be updated about the MiMa check: https://github.com/playframework/play-json/actions/runs/8133336570/job/22224801331?pr=993#step:6:226

@cchantep Please see my comment above :)

About binary compatibility – there is another approach we can take that would be binary compatible: keep mapWrites where it is but drop the implicit keyword. That breaks source compatibility, but only when it is imported explicitly so the impact would be minimal. Let me know what you think!

@cchantep
Copy link
Member

cchantep commented Mar 5, 2024

Seems it needs to be updated about the MiMa check: https://github.com/playframework/play-json/actions/runs/8133336570/job/22224801331?pr=993#step:6:226

@cchantep Please see my comment above :)

About binary compatibility – there is another approach we can take that would be binary compatible: keep mapWrites where it is but drop the implicit keyword. That breaks source compatibility, but only when it is imported explicitly so the impact would be minimal. Let me know what you think!

As soon as we speak of 2.8, without backport, I'm fine with either excluding the MiMa warn in build or keeping the function without implicit.

…String, T]]

Moved the deprecated method `DefaultWrites.mapWrites` into `LowPriorityWrites` to get rid of the ambiguity.
@sgodbillon sgodbillon force-pushed the sgodbillon/974-seq-of-map-aambiguous-implicits-scala3 branch from 1107a14 to da637a8 Compare March 5, 2024 16:41
@sgodbillon
Copy link
Contributor Author

As soon as we speak of 2.8, without backport, I'm fine with either excluding the MiMa warn in build or keeping the function without implicit.

Yes, that's only for 2.10. I opted for removing the implicit keyword

@cchantep cchantep enabled auto-merge (squash) March 5, 2024 17:50
@cchantep cchantep merged commit a842faf into playframework:main Mar 5, 2024
14 checks passed
@mkurz
Copy link
Member

mkurz commented Mar 7, 2024

As soon as we speak of 2.8, without backport, I'm fine with either excluding the MiMa warn in build or keeping the function without implicit.

Yes, that's only for 2.10. I opted for removing the implicit keyword

Sorry I don't follow, should this fix backported to 2.10.x and be part of a 2.10.5 release?

@mkurz
Copy link
Member

mkurz commented Apr 29, 2024

OK, if I understood correctly you want that be part of 2.10.5, so let's backport.

@mkurz
Copy link
Member

mkurz commented Apr 29, 2024

@Mergifyio backport 2.10.x

Copy link
Contributor

mergify bot commented Apr 29, 2024

backport 2.10.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Apr 29, 2024
…993)

Moved the deprecated method `DefaultWrites.mapWrites` into `LowPriorityWrites` to get rid of the ambiguity.

(cherry picked from commit a842faf)
mkurz added a commit that referenced this pull request Apr 29, 2024
[2.10.x] Fix #974 – Scala 3 macro cannot find Writes for Seq[Map[String, T]] (backport #993) by @sgodbillon
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.

Scala 3 macro cannot find Writes for Seq[Map[String, T]]
3 participants