bitcrowd
Format project with Elixir's code formatter
Regarding maintainability and easy headstart for new developers, it could make sense to format the whole project with Elixir's "new" code formatter.
After this is done, we could even add an additional step to CI for checking this as @pmeinhardt suggested:
mix format --check-formatted
I initially thought this might be a quick thing and whipped up a PR: https://github.com/bitcrowd/sshkit.ex/pull/125
Now I'm not so sure how to deal with different versions of Elixir (e.g. our build matrix on Travis CI) wanting different formats (or not having mix format at all, like Elixir <1.5).
Any ideas on how to handle this are welcome. Should we just check formatting for the latest version in our build matrix?
It's probably enough to check both, credo and formatting only for the latest versions. All other are meant to slowly move out of the matrix anyways 🗑
This only adds the implicit assumption that people contributing to the project are able to run it with a new enough Elixir version locally. And that's not asked too much I'd say.
For the code itself it won't make a difference anyways, right? I could not imagine formatting to the rules of a newer version actually breaking code for older versions... 🤔
The code shouldn't break, but mix format --check-formatted and thus CI might fail if the formatting preferences change between different Elixir versions I guess.
So running credo and the formatting check only for the latest version should be more or less "save".
IMO the great thing about mix format is that it allows us to move away from CI checks (i.e., --check-formatted) to a more user-friendly automated way of enforcing formatting. In another project we had set up an "autoformatbot" that would be triggered on push to master and would, in case master wasn't formatted properly, open a friendly follow-up PR. It could have even pushed to master directly, in my view, but we wanted to keep some control over it. We had it running on GitLab though.
Hey @maltoe, it's been a while but the problem was that the formatting produced by mix format and expected by mix format --check-formatted differs between Elixir versions. However, since SSHKit is not an application but a package/library, we're testing across multiple Elixir/Erlang/OTP versions. This means that formatting in a what that passes for all tested versions is/can be impossible (see #125).
For now, we just stuck with the Credo checks (which also check more than plain formatting, so some of them will be useful regardless of whether we use mix format or not).
Not sure whether formatting (and checking formatting) only for the most recent Elixir version would be a good idea?
Hey @pmeinhardt, I think we need to differentiate between auto-formatting and checking formatting in PRs more clearly.
With regards to auto-formatting, I don't see a reason why using latest Elixir to format the code should break the use of the library in older Elixir versions. Only maybe if there was a breaking syntax change introduced into a future Elixir version (as in, AST changes completely, and mix format produces weird code that Elixir 1.5 doesn't understand), but do you think this is likely?
My point about checking formatting - where this different Elixir version problem surely exists - was that maybe we could get away with not doing it at all. At least I liked this friendly post-processing workflow in the other project a lot, definitely more than getting a CI fail when I forgot to run mix-format. However, that was an internal project, so contributors were aware already that they were supposed to auto-format pre-commit. Of course, such a system does not have the same educational effect as CI fails...
p.s.: First issue discussion, hooray! And it is on formatting :smile: I looked into the code btw., and it looks really nice and clean :heart_eyes:. Though it seemed you have started working on many of the issues already, so I didn't really dive into coding.
Ah, I think now I get it. I feel a bit slow today. 😄 So what you're saying is you mean to apply "auto-formatting" in some automated way but not checking code format in CI? 🤔
To me personally, it feels a bit awkward to merge changes into master and then having a bot open PRs/push to master to fix the formatting. It sounds like a lot of noise/extra diffs in the commit history. 🤷♂️
But then again, you're absolutely right. With multiple Elixir versions, "manual" formatting requires you to use a specific version while formatting, which really doesn't sound great.
How do other, popular package/library projects solve this? (Unfortunately I don't have time to look into it right now)
