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

Use OCamlformat? #6

Open
lindig opened this issue Oct 5, 2021 · 3 comments
Open

Use OCamlformat? #6

lindig opened this issue Oct 5, 2021 · 3 comments

Comments

@lindig
Copy link
Owner

lindig commented Oct 5, 2021

Ho do you feel about re-indenting all code using OCamlformat and introducing a make format targe for this? I'm using it now on all my current projects and it has changed my opinion about indentation. Yes, you can emphasize meaning by using alignment that an automatic tool can't create but I think the advantages of consistent indentation outweigh this. In particular tricky cases (if vs. match) where indentation can become misleading after changes.

@dmbaturin
Copy link
Collaborator

My main concern about formatting tools in established projects is the loss of revision history. It becomes impossible to trace a line to a specific commit using git blame if the last non-cosmetic change was before the introduction of the formatter.

So I wonder if history loss would be an acceptable price to pay for consistent indentation. There are also problems other than indentation now, such as chunks of commented-out code. Ideally "all good people of the world" should give that codebase a good review and clean-up.

If we can get anyone else involved in that, it may be a good idea to run the code through the formatter before the clean-up project begins because so far most changes are attributed to either you, me, or two more people (@Drup and @Leonidas-from-XIV).

@Leonidas-from-XIV
Copy link
Contributor

Dune supports reformatting out of the box with dune build @fmt there is not much that needs to be done to set it up. I myself have become a big fan of automatic formatting to the point where I usually don't even try to write sensible code, I just write whatever and let the formatter handle it looking consistent.

That said, I understand @dmbaturin's concern. There is no good solution for this, but as a stopgap it is possible to ignore certail revs for git blame. The downside is, that it doesn't automatically work on remote repos.

@lindig
Copy link
Owner Author

lindig commented Oct 22, 2021

Thanks for the comments. I understand @dmbaturin's concern as well. Given that the code doesn't see a lot of changes, I would argue that it is a good moment to do this, hoping to benefit from it in the future by making changes easier. Is git blame right now a frequent use case? I would doubt it and would still be in favour as another step in modernising the code base.

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

No branches or pull requests

3 participants