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

Correct version guard for dune formatting change #11486

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

nojb
Copy link
Collaborator

@nojb nojb commented Feb 18, 2025

The formatting change introduced in #10892 was guarded under version 3.18 in #11175 to avoid breaking existing users of the unversioned dune format-dune-file (see #10892 (comment)). However, its replacement action (format-dune-file) introduced #11166 will only appear in 3.18, so the format change should in fact be guarded under version 3.19 to give users time to adapt.

@Leonidas-from-XIV
Copy link
Collaborator

Sounds fair enough, especially as it would be nice to release 3.18 soon-ish. Can you promote the failures?

@nojb
Copy link
Collaborator Author

nojb commented Feb 18, 2025

Sounds fair enough, especially as it would be nice to release 3.18 soon-ish. Can you promote the failures?

I promoted some of the failures, but I cannot reproduce the rest in my machine. Can you help me by pushing the corrected test files to this branch? Thanks!

@Leonidas-from-XIV Leonidas-from-XIV force-pushed the dune_format_changes_version_fix branch from f2434d4 to e321b7a Compare February 18, 2025 14:40
@Leonidas-from-XIV
Copy link
Collaborator

I was confused about this as well as it worked for me too. But the issue was that I had too old of a ppxlib in my dev-switch so I've created #11488.

nojb added 2 commits February 18, 2025 18:07
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
@nojb nojb force-pushed the dune_format_changes_version_fix branch from e321b7a to f42b812 Compare February 18, 2025 20:39
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
@nojb nojb force-pushed the dune_format_changes_version_fix branch from f42b812 to 11d0f1c Compare February 18, 2025 20:39
@nojb
Copy link
Collaborator Author

nojb commented Feb 18, 2025

I was confused about this as well as it worked for me too. But the issue was that I had too old of a ppxlib in my dev-switch so I've created #11488.

Thanks, now almost everything passes. However there's still one failure that looks unrelated. Do you confirm?

Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

Yes, the Dynlink failure happens (more than) occasionally but is unrelated.

@nojb nojb merged commit 04f28da into ocaml:main Feb 19, 2025
24 of 25 checks passed
@nojb nojb deleted the dune_format_changes_version_fix branch February 19, 2025 08:58
@nojb
Copy link
Collaborator Author

nojb commented Feb 19, 2025

Thanks for the confirmation!

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