[PM-26387] Fix lint warnings #2007
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2007 +/- ##
==========================================
- Coverage 81.32% 75.78% -5.55%
==========================================
Files 832 1018 +186
Lines 52457 62052 +9595
==========================================
+ Hits 42663 47028 +4365
- Misses 9794 15024 +5230 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ), | ||
| ], | ||
| name: "example.com", | ||
| name: "example.com" |
There was a problem hiding this comment.
🤔 For some reason, I thought we had final commas turned on? Did that change? And if this is just for parameter lists, I kind of think we should be consistent about it, especially now that Xcode 26 allows it. Thoughts?
There was a problem hiding this comment.
I personally don't like trailing commas when there are no further items after that (nor see the benefit of it). I can live with the ones in the array lists as there are a lot of them and it'd be tedious to change them all, but IMO we shouldn't use that for function parameters (as in this case).
There was a problem hiding this comment.
Ah, I prefer trailing commas because it makes diffs easier to read and makes it easier to rearrange the orders of things (or insert/delete items), because you never have to futz with the commas as well.
There was a problem hiding this comment.
For some reason, I thought we had final commas turned on?
I think SwiftFormat didn't previously complain about whether you included the trailing comma or not for parameter lists. It is configurable though, so we could enable either option.
I do like trailing commas for the reasons @KatherineInCode noted. Parameter lists would take some getting used to, but it might be nice for consistency. That would be a large change though, so maybe we can consider it and enable it separately if we want to do that.
There was a problem hiding this comment.
It might be reasonable to just turn the rule off entirely right now, and address the inconsistency later after some discussion?
There was a problem hiding this comment.
Oh that's a nice thought, I don't mind dealing with the commas but if it's easier for review and code rearrangement then let's configure and use it for sure 👍
🎟️ Tracking
PM-26387
📔 Objective
Fixes the remaining lint warnings.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes