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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the need to indent code blocks (sample) #14

Closed
wants to merge 1 commit into from

Conversation

swiftal64
Copy link
Contributor

Summary

Pros:

  • We no longer have to indent code blocks.

Cons:

  • We no longer have bold and non-bold sections in rule titles.

Alternatives considered:

  • Instead of using a heading on the rules, use just standard bold. This causes the "details" link to hug the next rule and doesn't provide the spacing we'd like.

Reasoning

Reviewers

cc @airbnb/swift-styleguide-maintainers

Please react with 馃憤/馃憥 if you agree or disagree with this proposal.

Pros:
- We no longer have to indent code blocks.

Cons:
- We no longer have bold and non-bold sections in rule titles.

Alternatives considered:
- Instead of using a heading on the rules, use just standard bold. This causes the "details" link to hug the next rule and doesn't provide the spacing we'd like.
@dfed
Copy link
Contributor

dfed commented Sep 4, 2018

I'm not sure I understand what's going on here: what problem are we solving? Can we get a little more context?

@bachand
Copy link
Contributor

bachand commented Sep 4, 2018

Here is what the visual diff looks like:
a848e112-a72e-4fef-8d6b-0a9b123f0ad3

@fdiaz
Copy link
Collaborator

fdiaz commented Sep 4, 2018

I feel like contributing is a bit harder (since you have to indent 2 spaces your code blocks), but we get better legibility by keeping the current style.

I feel like the con of not having bold/non-bold sections is a fairly big disadvantage compared to what we gain. I think we're trading the ease of reading the guide for multiple people by focusing on making life a bit easier to a couple of contributors.

Should we move this to an issue to discuss since this PR is not merge-ready based on our contributing guidelines and if we agree on doing this, then we can make a PR that moves the style guide in that direction?

@bachand
Copy link
Contributor

bachand commented Sep 4, 2018

@fdiaz can you clarify what you mean by:

I feel like the con of not having bold/non-bold sections is a fairly big disadvantage compared to what we gain

We still have bold with this new approach since we鈥檙e using a header style instead of explicit bold, right?

Also happy to discuss further in an issue.

@fdiaz
Copy link
Collaborator

fdiaz commented Sep 5, 2018

We still have bold with this new approach since we鈥檙e using a header style instead of explicit bold, right?

Correct. But we only have bold. We can't have non-bold parts of the title so it all "looks" like the same weight when in reality IMO there are more relevant parts of the rule and less relevant. For example, the sentence of the rule should IMO be more important and highlighted than the SwiftLint part. Same with the link at the beginning. Making everything equal looses the focus on the rule itself IMHO.

@bachand
Copy link
Contributor

bachand commented Sep 6, 2018

@fdiaz, got it. Makes sense. Thanks for clarifying.

@bachand
Copy link
Contributor

bachand commented Sep 6, 2018

I like not indenting but I agree it would be nice to use bold sparingly.

@thedrick
Copy link

I agree with the points that Francisco has made here and I personally prefer the current style compared to the proposal.

@fdiaz
Copy link
Collaborator

fdiaz commented Sep 14, 2018

Resolution

Status: Declined 鉀旓笍

Reasoning:

This proposal made it easier to contribute to our Style Guide but (arguably) would have made it harder to read given that the rule would have been all in bold.

@fdiaz fdiaz closed this Sep 14, 2018
@fdiaz fdiaz deleted the oy/unindent-code-blocks branch September 14, 2018 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants