sweet_xml icon indicating copy to clipboard operation
sweet_xml copied to clipboard

Fix warnings for Elixir 1.17 by formatting the project using `mix format`

Open wkirschbaum opened this issue 1 year ago • 3 comments

This PR adds a .formatter.exs file and applies changes when running mix format. Additionally mix deps.update --all has been run to ensure we don't receive warnings for older versions of dependencies.

wkirschbaum avatar Jun 04 '24 06:06 wkirschbaum

Hi @wkirschbaum :wave:

Thank you for you contribution, it is very much appreciated !

However, we avoid using mix's formatter for repos that don't use it from the beginning, reason being that the changes they apply don't have any functionnal meaning, and we try to keep an as obvious and simple as possible in the git history.

I know it is not obvious on the repository scope. We currently are in the process of smoothing out all of our public repos, and as such they don't all have a "Contributing" section in their README.md yet, or a link the our organization's one, where we have a section related to how we handle changes related to formatting commits (relevant section here).

I'll be more than happy to merge your PR with the two formatting-related commits dropped.

Thanks again !

half-shell avatar Jun 12 '24 14:06 half-shell

@half-shell thanks for the maintenance of this library.

The formatting is to get rid of the warnings. Either we format it manually, or we use the tool for less risk of messing something up. if you want to manually only change the '' to ~c"", but unfortunately I don't feel it is time well spent.

The two formatting commits is the purpose of this PR, so omitting it defeats the point 😆.

I won't be offended if you reject this PR :), then perhaps this PR can be viewed as an issue instead to solve warnings for 1.17.

wkirschbaum avatar Jun 12 '24 15:06 wkirschbaum

You're right, and I should have been clearer in my answer.

What I meant was to avoid mass formatting over a whole repo. The warnings you mention only relate to 5 lines, so what I'm arguing for is rather to have a commit "remove several warnings regarding the use of charlists instead of strings" with only those lines changed :slightly_smiling_face:

half-shell avatar Jun 13 '24 08:06 half-shell