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

Add unique identifiers to each rule #13

Closed
wants to merge 1 commit into from
Closed

Conversation

thedrick
Copy link

@thedrick thedrick commented Aug 31, 2018

Summary

This removes the (link) from every rule and replaces it with a unique identifier (following the numerical conventions from our Javascript style guide https://github.com/airbnb/javascript).

Reasoning

The benefit of doing this is that when you reference a style guide rule, you can reference it by number instead of pasting in the entire rule / a sentence. e.g. "This violates rule 3.4 of the style guide" instead of "This violates long function invocations should also break on each argument". I find this makes the style guide easier to scan as well

Cons of this: updating rules that come earlier is a huge pain since you have to update every rule after it.

Reviewers

cc @airbnb/swift-styleguide-maintainers

Please react with 👍/👎 if you agree or disagree with this proposal.

This removes the (link) from every rule and replaces it with a unique identifier (following the numerical conventions from our Javascript style guide https://github.com/airbnb/javascript)

The benefit of doing this is that when you reference a style guide rule, you can reference it by number instead of pasting in the entire rule / a sentence. e.g. "This violates rule 3.4 of the style guide" instead of "This violates 'long function invocations should also break on each argument'"
@thedrick thedrick requested a review from fdiaz August 31, 2018 21:22
@swiftal64
Copy link
Contributor

swiftal64 commented Sep 4, 2018

I do agree that "This violates long function invocations should also break on each argument" is too long to type. However, I don't think referring to them with numbers is a good idea because rules could move around. So if you refer to "this violates rule 1.3" for example, that may be valid only for a week if a new rule gets added before 1.3. So the link will say 1.3, but it'll point to 1.4, causing an incongruence in what is typed and where it links to.

This is the main reason we chose identifiers instead of numbers in the first place.

We need to keep in mind that, especially if this becomes open source and popular, there will be tons of links to specific rules that live around for many years. It's not reasonable to ask everyone to update the number in their links whenever the style guide changes. Instead it would be best if rule identifiers never changed.

What about instead of referring to the long identifier or the number, you say something like "This violates #long-function-invocation," and we make it easy to see/copy that name?

For example:

(#long-function-invocation) Long function invocations should also break on each argument. Put the closing parenthesis on the last line of the invocation. SwiftLint: multiline_arguments vertical_parameter_alignment_on_call

@fdiaz
Copy link
Collaborator

fdiaz commented Sep 4, 2018

I agree with Ortal's points but IMHO we don't need to change the structure of our style guide. I've been using:

This violates this rule

Which is short, simple and it has a permalink so even if the rule moves around in the style guide it will still point to the correct link.

I don't see a need to either use numbers or the identifier when linking to the rule 🤷‍♂️

@@ -43,15 +43,15 @@ _You can enable the following settings in Xcode by running [this script](resourc

</details>

* <a id='spaces-over-tabs'></a>(<a href='#spaces-over-tabs'>link</a>) **Use 2 spaces to indent lines.**
* <a id='spaces-over-tabs'></a><a href='#spaces-over-tabs'>1.2</a> **Use 2 spaces to indent lines.**
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i strongly suggest never using numbers whatsoever; they're extremely brittle as you add, remove, or re-order sections.

@thedrick
Copy link
Author

thedrick commented Sep 4, 2018

I have been sufficiently convinced, I do really like ortal's proposal of giving each rule a shorthand name that could be referenced, but we can propose that in a separate PR. I do still think the proliferation of (link) at the beginning of every rule makes the document hard to parse

@thedrick thedrick closed this Sep 4, 2018
@ljharb ljharb deleted the thedrick-unique-ids branch September 4, 2018 17:34
@swiftal64
Copy link
Contributor

@thedrick what about if instead of "(link)" it was "#" or something else that's subtle and could act like a functional bullet point?

@swiftal64
Copy link
Contributor

Or even ⚓️

@fdiaz
Copy link
Collaborator

fdiaz commented Sep 11, 2018

Resolution

Status: Declined ⛔️

Reasoning:

  • Following the numerical conventions from our Javascript style guide would make the Style Guide extremely brittle when adding, removing, or re-ordering sections.

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.

4 participants