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

[new release] ppx_deriving 6.0.2 #25918

Merged
merged 3 commits into from
May 28, 2024

Conversation

NathanReb
Copy link
Contributor

No description provided.

@NathanReb
Copy link
Contributor Author

NathanReb commented May 22, 2024

Let's hope this one works! I opened a draft this time to test out the revdeps builds and added upper bounds to pa_ppx as mentioned in #25720 (comment)

@NathanReb
Copy link
Contributor Author

There's a lot of request time out in the CI. Outside of those I couldn't spot any errors related to ppx_deriving but I might have missed something. @mseri does anything seem fishy to you here?

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@mseri mseri force-pushed the release-ppx_deriving-6.0.2 branch from ca18d5e to bfc383b Compare May 24, 2024 09:47
@mseri
Copy link
Member

mseri commented May 24, 2024

I fixed the merge conflict. I'll let the CI re-run, hopefully we will see less timeouts. I think it's fine to merge

@NathanReb
Copy link
Contributor Author

Awesome, thanks! I'll go through with the actual release and undraft!

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@NathanReb NathanReb marked this pull request as ready for review May 24, 2024 10:15
@mseri
Copy link
Member

mseri commented May 28, 2024

I think this release or one of its dependencies has dropped a dependency on result. I'll send a PR to fix all the broken stuff before merging, sorry for the extra delay this will add

@mseri
Copy link
Member

mseri commented May 28, 2024

Bounds added in #25951

I think this are safe to add, since they were using result in their code but taking it as transitive dependence from ppx_deriving

@NathanReb
Copy link
Contributor Author

Wouldn't the fix be to add the result dependency instead? That way they could still build with ppx_deriving.

No worries for the delay, I forgot to come back and check if they were new failures so that's partly on me.

@mseri
Copy link
Member

mseri commented May 28, 2024

Wouldn't the fix be to add the result dependency instead? That way they could still build with ppx_deriving.

No because they don't link it explicitly, they were taking it in as transitive dependency

@NathanReb
Copy link
Contributor Author

Ah yes you're right! My bad!

@mseri
Copy link
Member

mseri commented May 28, 2024

Looks good now. The timeouts are a known issue that is being investigated and does not depend on this

@mseri mseri merged commit 7ac1012 into ocaml:master May 28, 2024
1 of 2 checks passed
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.

None yet

2 participants