Skip to content

Correct version guard for dune formatting change#11486

Merged
nojb merged 3 commits intoocaml:mainfrom
nojb:dune_format_changes_version_fix
Feb 19, 2025
Merged

Correct version guard for dune formatting change#11486
nojb merged 3 commits intoocaml:mainfrom
nojb:dune_format_changes_version_fix

Conversation

@nojb
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

@nojb
Copy link
Copy Markdown
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
Copy Markdown
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 <n.oje.bar@gmail.com>
Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
@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 <n.oje.bar@gmail.com>
@nojb nojb force-pushed the dune_format_changes_version_fix branch from f42b812 to 11d0f1c Compare February 18, 2025 20:39
@nojb
Copy link
Copy Markdown
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
Copy Markdown
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
@nojb nojb deleted the dune_format_changes_version_fix branch February 19, 2025 08:58
@nojb
Copy link
Copy Markdown
Collaborator Author

nojb commented Feb 19, 2025

Thanks for the confirmation!

anmonteiro pushed a commit to anmonteiro/dune that referenced this pull request Apr 22, 2025
Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
Sudha247 pushed a commit to Sudha247/dune that referenced this pull request Jul 23, 2025
Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
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