chore: experiment with swift-format for PR#174
Conversation
|
@FL33TW00D, maybe try a line length of 120, which might result in fewer changes and looks better to my eye. If it's making too many changes that are not to your taste, it might be because it's applying default settings that you can override by specifying rules. |
|
@DePasqualeOrg if you have any suggestions on config to try and minimize changes that would be great! The main one for me is how to disable adding |
|
It looks like the I don't care too much about specific rules, as long as they don't make the code harder to read. I find the 100-character line length makes the code less readable, and I have my configuration files set to 120 for that reason. |
|
@DePasqualeOrg Updated to 120 line length. This can be tested locally with |
|
Of course the best time to start using formatting would have been at the beginning of the project, to avoid having changes later on. But I think the benefits of having automated formatting outweigh any drawbacks (that I'm aware of) of this one-time change. |
|
After discussions with @ZachNagengast, it seems that nicklockwoods I'd be happy to format the whole repo using this, but will defer to @pcuenca for confirmation. Thanks @ZachNagengast! |
|
If finalized, consider adding a |
|
This issue has been in discussion for months, and the indecision continues to create problems for me as I try to contribute to this package. I would really appreciate it if you could decide on a solution and merge it. |
|
Sorry for the delay @DePasqualeOrg @pcuenca and myself will make sure some formatting solution is in by the end of the week. |
Good suggestion thank you! |
|
Overall I think we've decided to go with a gated CI approach using The suggestion from @shavit is solid, and we can add a note about the If @pcuenca agrees then hopefully we can go for it! |
pcuenca
left a comment
There was a problem hiding this comment.
Yes, CI formatting is enough imo, we can also provide a local script to run the steps locally if we need to.
Can we have all the changes applied here so we can see what the result looks like?
e5b6ce2 to
451ec9e
Compare
pcuenca
left a comment
There was a problem hiding this comment.
We already found:
- A compilation problem because one of the rules (using keypaths) should not have been applied.
- Blank space changes in literal Strings 😱
I hope this is useful to others, but it sure is scarier than it should (and a time sink already).
|
Are you referring to the multi-line string in |
|
swift-formatseems quite heavy handed.