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

Enable ocamlformat on the part of the codebase that doesn't use CPPO #153

Merged

Conversation

Leonidas-from-XIV
Copy link
Member

This enables ocamlformat on the subset of the codebase that does not use CPPO.

@Leonidas-from-XIV Leonidas-from-XIV added the no changelog Not a user visible change, does not require changelog entry label Aug 15, 2022
@Leonidas-from-XIV Leonidas-from-XIV mentioned this pull request Aug 22, 2022
Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

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

I approve. I would also suggest adding a .git-blame-ignore-revs so that git blame output more meaningful result.

Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

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

In fact, currently not only .cppo.ml contains cppo instructions, see for instance monomorphic.ml.

They should either be renamed, or added to the ocamlformat-ignore!

@Leonidas-from-XIV
Copy link
Member Author

Leonidas-from-XIV commented Aug 22, 2022

Oh yes, that's true, you're right. I somehow assumed only the .cppo. files had CPPO in them, so I added them to the ignore-file to avoid churn. Turns out ocamlformat didn't reformat them anyway (so I didn't have to undo anything), maybe it has gotten sufficiently smart to skip them. Still good to have it skip over.

Also, I added the revision of the commit to the file. Unfortunately, there's no default name that git would pick up and users need to configure git for this to work but adding it won't hurt.

Can you please take another look?

Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

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

Looks good!

Also, I added the revision of the commit to the file. Unfortunately, there's no default name that git would pick up and users need to configure git for this to work but adding it won't hurt.

I think .git-blame-ignore-revs is the name that github picks, so it's at least directly useful in the github interface. (https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view)

@Leonidas-from-XIV
Copy link
Member Author

Ah, that's great to know, definitely useful!

@Leonidas-from-XIV Leonidas-from-XIV merged commit a3fbc94 into ocaml-community:master Aug 22, 2022
@Leonidas-from-XIV Leonidas-from-XIV deleted the ocamlformat-enable branch August 22, 2022 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Not a user visible change, does not require changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants