Skip to content
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

Print YAML configuration in output of rules command #5113

Merged
merged 3 commits into from
Aug 28, 2023

Conversation

SimplyDanny
Copy link
Collaborator

I wonder if this is actually worth a separate swiftlint config [--default] <rule> command? 🤔

@SimplyDanny SimplyDanny requested a review from jpsim July 9, 2023 21:07
@SwiftLintBot
Copy link

SwiftLintBot commented Jul 9, 2023

17 Messages
📖 Linting Aerial with this PR took 1.12s vs 1.13s on main (0% faster)
📖 Linting Alamofire with this PR took 1.47s vs 1.47s on main (0% slower)
📖 Linting Brave with this PR took 8.39s vs 8.41s on main (0% faster)
📖 Linting DuckDuckGo with this PR took 4.13s vs 4.14s on main (0% faster)
📖 Linting Firefox with this PR took 9.81s vs 9.88s on main (0% faster)
📖 Linting Kickstarter with this PR took 10.63s vs 10.62s on main (0% slower)
📖 Linting Moya with this PR took 0.59s vs 0.61s on main (3% faster)
📖 Linting NetNewsWire with this PR took 3.18s vs 3.19s on main (0% faster)
📖 Linting Nimble with this PR took 0.78s vs 0.78s on main (0% slower)
📖 Linting PocketCasts with this PR took 8.36s vs 8.36s on main (0% slower)
📖 Linting Quick with this PR took 0.38s vs 0.38s on main (0% slower)
📖 Linting Realm with this PR took 11.92s vs 11.94s on main (0% faster)
📖 Linting Sourcery with this PR took 2.4s vs 2.39s on main (0% slower)
📖 Linting Swift with this PR took 5.07s vs 5.1s on main (0% faster)
📖 Linting VLC with this PR took 1.43s vs 1.42s on main (0% slower)
📖 Linting Wire with this PR took 8.71s vs 8.73s on main (0% faster)
📖 Linting WordPress with this PR took 12.55s vs 12.57s on main (0% faster)

Generated by 🚫 Danger

@SimplyDanny SimplyDanny force-pushed the print-yaml branch 2 times, most recently from beb3f32 to 74cd24f Compare July 9, 2023 21:57
@mildm8nnered
Copy link
Collaborator

I wonder if this is actually worth a separate swiftlint config [--default] <rule> command? 🤔

So not arguing against the concept, but I actually had my eye (but not my heart set) on configure as a new command for a Periphery style configuration wizard - see #4898 and #5041 - happy to pick an alternative, but if we had config and configure, that would potentially be a bit confusing.

@SimplyDanny
Copy link
Collaborator Author

I wonder if this is actually worth a separate swiftlint config [--default] <rule> command? 🤔

So not arguing against the concept, but I actually had my eye (but not my heart set) on configure as a new command for a Periphery style configuration wizard - see #4898 and #5041 - happy to pick an alternative, but if we had config and configure, that would potentially be a bit confusing.

Good point. My idea is to offer a way to append a default configuration for a rule to the .swiftlint.yml file like swiftlint config <rule> >> .swiftlint.yml. However, I also see this could be added to the rules command by adding two new options to it:

  • --config-only to print only the rule's configuration as YAML
  • --default to use the rule's default configuration (instead of what's configured) wherever it is printed

I think the --default also makes sense without the --config-only option for this command.

@SimplyDanny SimplyDanny force-pushed the print-yaml branch 3 times, most recently from d1d0beb to a68f204 Compare July 17, 2023 21:20
@SimplyDanny SimplyDanny enabled auto-merge (rebase) August 28, 2023 18:03
@SimplyDanny SimplyDanny merged commit 84405f9 into realm:main Aug 28, 2023
@SimplyDanny SimplyDanny deleted the print-yaml branch August 28, 2023 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants