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

Document modified configurations used for examples #5402

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SimplyDanny
Copy link
Collaborator

@SimplyDanny SimplyDanny commented Dec 23, 2023

Example:

for user in users {
  if user.id == 1 { return true }
}

//
// allow_for_as_filter: true
//

@SwiftLintBot
Copy link

SwiftLintBot commented Dec 23, 2023

17 Messages
📖 Linting Aerial with this PR took 0.82s vs 0.81s on main (1% slower)
📖 Linting Alamofire with this PR took 1.2s vs 1.15s on main (4% slower)
📖 Linting Brave with this PR took 6.58s vs 6.61s on main (0% faster)
📖 Linting DuckDuckGo with this PR took 3.57s vs 3.53s on main (1% slower)
📖 Linting Firefox with this PR took 8.95s vs 9.01s on main (0% faster)
📖 Linting Kickstarter with this PR took 8.0s vs 8.0s on main (0% slower)
📖 Linting Moya with this PR took 0.48s vs 0.47s on main (2% slower)
📖 Linting NetNewsWire with this PR took 2.48s vs 2.48s on main (0% slower)
📖 Linting Nimble with this PR took 0.66s vs 0.69s on main (4% faster)
📖 Linting PocketCasts with this PR took 6.78s vs 6.84s on main (0% faster)
📖 Linting Quick with this PR took 0.33s vs 0.35s on main (5% faster)
📖 Linting Realm with this PR took 4.0s vs 3.94s on main (1% slower)
📖 Linting Sourcery with this PR took 2.09s vs 2.1s on main (0% faster)
📖 Linting Swift with this PR took 3.86s vs 3.91s on main (1% faster)
📖 Linting VLC with this PR took 1.09s vs 1.11s on main (1% faster)
📖 Linting Wire with this PR took 14.18s vs 14.21s on main (0% faster)
📖 Linting WordPress with this PR took 9.39s vs 9.42s on main (0% faster)

Generated by 🚫 Danger

@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch from f875199 to 7f01e38 Compare December 24, 2023 13:02
@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch 2 times, most recently from 16b7bf2 to f0abd25 Compare January 9, 2024 21:47
@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch from f0abd25 to 246619b Compare January 23, 2024 21:11
@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch from 246619b to 371e38b Compare February 6, 2024 21:58
@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch 3 times, most recently from da6c09b to 671bfc3 Compare March 9, 2024 17:43
@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch from 671bfc3 to 4dd2f3e Compare April 7, 2024 17:14
@SimplyDanny SimplyDanny force-pushed the document-modified-configurations branch from 4dd2f3e to 539c811 Compare April 7, 2024 17:28
@SimplyDanny
Copy link
Collaborator Author

@mildm8nnered: I would like to hear your opinion on this. Is the presentation style clear enough?

@mildm8nnered
Copy link
Collaborator

@mildm8nnered: I would like to hear your opinion on this. Is the presentation style clear enough?

So I think it's a matter of taste, but I'd probably put the context before the example, so something like this:

// allow_for_as_filter: true
for user in users {
  if user.id == 1 { return true }
}

although that could collide with swift documentation comments in some cases ...

I guess there's also a bit of a risk that people might think these are directives like // swiftlint:disable somehow, and end up pasting them into their own code.

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.

None yet

3 participants