Skip to content

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

Merged
cchantep merged 1 commit intoplayframework:mainfrom
sgodbillon:sgodbillon/974-seq-of-map-aambiguous-implicits-scala3
Mar 5, 2024
Merged

Fix #974 – Scala 3 macro cannot find Writes for Seq[Map[String, T]]#993
cchantep merged 1 commit intoplayframework:mainfrom
sgodbillon:sgodbillon/974-seq-of-map-aambiguous-implicits-scala3

Conversation

@sgodbillon
Copy link
Contributor

@sgodbillon sgodbillon commented Mar 3, 2024

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

sgodbillon commented Mar 4, 2024

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

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

@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

sgodbillon commented Mar 4, 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!

@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.

@sgodbillon
Fix playframework#974 – Scala 3 macro cannot find Writes for Seq[Map[…
da637a8 
…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

sgodbillon commented Mar 5, 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

@cchantep cchantep enabled auto-merge (squash) March 5, 2024 17:50
@cchantep cchantep merged commit a842faf into playframework:main Mar 5, 2024
@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

@mergify
Copy link
Contributor

mergify bot commented Apr 29, 2024 •
edited
Loading

backport 2.10.x

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Apr 29, 2024
@sgodbillon @mergify
Fix #974 – Scala 3 macro cannot find Writes for Seq[Map[String, T]] (#…
eed1458 
…993)

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

(cherry picked from commit a842faf)
@mergify mergify bot mentioned this pull request Apr 29, 2024
Merged
5 tasks
mkurz added a commit that referenced this pull request Apr 29, 2024
@mkurz
Merge pull request #1022 from playframework/mergify/bp/2.10.x/pr-993
01b3cce 
[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

Reviewers

@cchantep cchantep cchantep approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

3 participants

@sgodbillon @cchantep @mkurz