-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Report and Improvements on Swiftlint baseline #5597
Comments
Hi @aiKrice - I replied to your comment here - #5475 (comment), but I'll reply here as well. Firstly, thank you so much for the feedback, which is incredibly useful (and also for the Medium articles - it is great to get the word out there). Coming back to your specific issues:
You need to use the same config file when you generate the baseline with This should work, as in both cases,
It's maybe because this file was listed as excluded in my config file" - it is exactly that.
You can generate a baseline with The reason that is mentioned in #5553 is more as a way of saying - "these are all our possible violations, let's pick which rules from here we would like to enable".
I agree that this can be a bit annoying and unhelpful when trouble-shooting. During development, I did turn pretty printing on, and in the Periphery implementation of the baseline feature I currently have pretty-printing on. The reason I turned this off was because of concerns about baseline file size. Not so much in the context of a single baseline, but more in the context of updating baselines under source control where they will occupy space in the git history. With pretty printing, the size of the file is increased by about ~25% (ballpark, I don't have exact figures offhand). Originally we were going to recommend compressing the baseline in the repo, and uncompressing it before use, or even having SwiftLint compress and uncompress the file itself. Having done some experiments with git history sizes - see #5553 (comment), it actually looks like its better to store it uncompressed, as that way In the absence of pretty-printing, you can use the
for a summary, or something like
for individual warnings. You could even run the latter a run script build phase, to see the Baseline violations reported directly within Xcode. In Periphery there is no equivalent of the Hope this helps - please let us know if things are not working the way you expected after this. Any feedback about bugs, usability, whether the documentation is clear enough etc. are very very welcome. |
Incidentally, I was very interested to see https://detekt.dev/docs/introduction/baseline/ and https://googlesamples.github.io/android-custom-lint-rules/usage/baselines.md.html which I think you mentioned somewhere, and which I was not aware of - I will definitely have a closer look at those. |
Hello @mildm8nnered , Thank for being so reactive 🚀 . 1️⃣. Baseline generation: 2️⃣. Baseline writing output. I was trapped ! I didn't understand that the output because it said: linting done! found violations and I didn't expect such an output which could lead me that the baseline failed. In this case, I suggest a verbose mode disabled by default or something else. Also I am wondering why if I add a file in the exclusion in my config.yml, it is taken in account in the baseline generation. It should be excluded (it is like this in Detekt and AndroidLint) 3️⃣ . Ok aligned of course. I did it because I though no choice 4️⃣ . If we could provide a pretty printing option it would be nice (i'm aligned of course on what you said). To sum it up, here are my opinions and suggestions : Overall. Nice !!! Gonna modify my Medium article with temp solution of recursive swift browsing. And available for discussing for Android equivalent. The friendly link medium article for how to generate the baseline for AndroidLint or Detekt in a nutshell is in an another article -> https://medium.com/@SaezChristopher/4-best-tools-to-implement-into-your-githooks-in-an-android-project-dfc93e1852b7?sk=a15ad91c046ab14f17614b7c43a486b0 |
So you should not have to provide your files one by one at all. For "medium-ish" side projects, for some value of "medium-ish" - mine is quarter of a million lines of Swift code, or thereabouts, enabling about 20 rules that I would have liked to turn on, but couldn't because of existing violations, the baseline is about 2.5MB. That is not really that large by modern standards. Even with I wasn't aware of a lot the prior art when I started, so I can't comment yet on Detekt and AndroidLint (but I am intending to check them out), but in summary, I would say, just invoke SwiftLint exactly how you would normally, with identical command lines in the writing (
So in SwiftLint's case, the baseline just says "ignore and do not report violations that are in the baseline" - there is no real extra output to say "we filtered these many violations". We could definitely look at adding something at least as a summary "0 violations detected, 6,107 violations filtered", but we don't do that right now. Reporting filtered violations in detail - that would be difficult. You might want to know "how many violations were filtered", or "how many violations in the baseline are not actually present in the tree any more". You can do that with the
Could you clarify that a bit, if my response to 1) above doesn't already cover that for you?
So Re-using the reporters for this is actually really nice, because a baseline is just a collection of violations, and we have multiple reporters (and can always add more) for reporting violations in various formats.
Thanks!
So maybe we could thrash out the issues here, and that might save you having to make multiple edits (and once again, thank you so much for popularizing this).
I am definitely going to check those out as soon as I have a chance! |
For info @mildm8nnered . I have performed:
For point 4, tried also, it works with the command. At least no need to install an additionnal tools. I will perform a last update of the article. Thank you 👍 |
So you should not need to do the Does that not work for you?
|
@mildm8nnered Do you see something wrong / missing in my config file above ?
I tried
And my team and I are running under zsh |
So I think it's not your config file per se, but where you've put it (in SwiftLint implicitly expects the main config file to be at the top of the tree, and then can use various mechanisms to refine the configuration. In your case, SwiftLint is probably trying to scan If you need to have the config file in |
New Issue Checklist
Describe the bug
I played this week end on with the baseline feature. First of all thank @mildm8nnered and all contributors, Amazing job and I am here also to help with my way, by trying hard.
I noticed several behavior that imho, are not exactly what I expected, as an Android developer as well, Detekt and AndroidLint baseline user.
1️⃣. I generate a baseline with this command:
I noticed a different behavior in the content of the baseline if I specifiy my config file
2️⃣. In both case above, I noticed that some rules some not ignored in the baseline so swiftlint failed. It was these rules :
It's maybe because this file was listed as excluded in my config file (see it below #githooks)
3️⃣. I generated a baseline with all rules enabled (as suggested in another issue opened) (but baseline should not be generated this way but against a configuration file), In this case "it works".
4️⃣. The baseline json file is a single 1 line file 😱 . In this case, Xcode (well, xcode...) cant read it. So I use a third party to format it: prettier -> https://github.com/prettier/prettier to format it. I think this should be opt-in because to compare this on a PR even to make it readable, it better. But by using prettier and an intermediate tool, there are some noises and too many illegitimates diffs.
Voila, hoping it can help, I am ready to kepp helping for improving it on my project 💪 .
Environment
version: 0.55.1
Homebrew, Mac M1PRO 14"
Xcode 15.4
Build version 15F31d
The text was updated successfully, but these errors were encountered: