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

Rule Request: Enforcing one space after slashes for comments #3233

Closed
2 tasks done
noahsark769 opened this issue Jun 10, 2020 · 13 comments
Closed
2 tasks done

Rule Request: Enforcing one space after slashes for comments #3233

noahsark769 opened this issue Jun 10, 2020 · 13 comments
Labels
rule-request Requests for a new rules.

Comments

@noahsark769
Copy link
Contributor

New Issue Checklist

New rule request

I looked around for this rule but didn't see it, so let me know if I missed it. It would be nice to have a rule which fails when inline comments (double-slash style) don't have one space between the double-slashes and the first text character.

Violations:

//something
    //something
//  something

Non-triggering examples:

// something
    // something

I think that most people feel that one space after the slashes is easier to read (though to be fair I don't have hard evidence, only anecdotal). All the Xcode templates and Apple sample code that I've seen, for example, use exactly one space after the slashes. Happy to make this an opt-in rule.

@christiankm
Copy link
Contributor

I think this could be fine, however it should enforce only for at least one space, as the commenter in Xcode will add the slashes in the start of the line.
Would you expect various //swiftlint:disable commands to also require one space, or should that not matter?

@noahsark769
Copy link
Contributor Author

noahsark769 commented Jun 23, 2020

Actually, I think I agree. There's also the idea of that you might want to indent a line in a docblock (///), which is further evidence for the at least one space way of thinking that I didn't consider before. I think that my preference would be for the swiftlint:disable comments to also require one space, but it's possible we could make that configurable.

@stale
Copy link

stale bot commented Nov 8, 2020

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

@stale stale bot added the wontfix Issues that became stale and were auto-closed by a bot. label Nov 8, 2020
@mozeryansky
Copy link

stale bot is bad bot

@stale stale bot removed the wontfix Issues that became stale and were auto-closed by a bot. label Nov 8, 2020
@jpsim
Copy link
Collaborator

jpsim commented Nov 8, 2020

@mozeryansky not sure if you want to engage in meaningful conversation about this, but the reality is that with the current availability of volunteer maintainers when it comes to reviewing PRs and triaging issues is quite low, so to help focus that energy, inactive issues will be closed automatically.

If you're looking for ways to help improve the situation, reviewing issues or PRs is a great way to help.

@mozeryansky
Copy link

I didn't see any maintainer say they won't address this issue or that it shouldn't be addresses, and I don't see any labels assigned to this ticket. I also would like this feature as part of SwiftLint as I manually maintain this rule within all my projects. It would would disappointing to see this issue pushed away and forgotten so I made a comment in order to get the bot to remove the "wontfix" tag.

Could the stale bot auto label issues as "needs attention" instead of "wontfix"?

@jpsim jpsim added the rule-request Requests for a new rules. label Nov 8, 2020
@jpsim
Copy link
Collaborator

jpsim commented Nov 8, 2020

I didn't see any maintainer say they won't address this issue or that it shouldn't be addresses, and I don't see any labels assigned to this ticket

The reason why I accepted to integrate the stalebot into this codebase in #3385 is that I don't actually have time to look through every issue and respond. So far I've spent about 14 hours today to start catching up on SwiftLint issues and PRs and I've barely made a dent in the outstanding issues and PRs that need attention or review.

So this bot is about setting reasonable expectations and keeping the scope for our current level of maintainership more feasible.

That being said, I think there's value in keeping rule requests around near-indefinitely, so I've made a change to the stalebot configuration to exempt rule requests from being closed as stale.

@mozeryansky
Copy link

Ok great, that makes sense. I wasn't sure the processes, just was mostly responding to the bot. Thanks for following up.

@jpsim
Copy link
Collaborator

jpsim commented Nov 8, 2020

The bot is actually not a person, but the maintainers of this project are.

@mozeryansky
Copy link

I know. A lot of repos use the same bot. Thanks for adding rule-request to the exemptLabels.

@noahsark769
Copy link
Contributor Author

Thank you both for the info! I'm hoping I can find some time myself to work on this soon. I'll update this issue with any progress

@jpsim
Copy link
Collaborator

jpsim commented Nov 8, 2020

Yes, sorry for the distraction, I'd also appreciate having a rule for comment spacing 🙏

noahsark769 added a commit to noahsark769/SwiftLint that referenced this issue Nov 8, 2020
noahsark769 added a commit to noahsark769/SwiftLint that referenced this issue Nov 9, 2020
noahsark769 added a commit to noahsark769/SwiftLint that referenced this issue Nov 10, 2020
noahsark769 added a commit that referenced this issue Nov 11, 2020
@noahsark769
Copy link
Contributor Author

Fixed by #3416, with the at least one space behavior 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

No branches or pull requests

4 participants