Clarify Rule: Use Swift's automatic enum values unless they map to an external source. #215

teddy5518 · 2023-03-21T15:19:36Z

Summary

This PR proposes to clarify comment of the "Use Swift's automatic enum values unless they map to an external source."

Reasoning

I indicated whether the examples are wrong or right like other rules in the guide to increase readability.
The origin comment was not being specific as it only said "swiftformat:enable redundantRawValues" and "swiftformat:disable redundantRawValues". The heading of rule provides link to swiftformat, so I thought it was better to directly indicate that the examples are right for relying on Swift's automatic enum values.

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

… external source.

calda · 2023-03-21T15:22:07Z

README.md

  /// These are written to a logging service. Explicit values ensure they're consistent across binaries.
-  // swiftformat:disable redundantRawValues


We should keep this example linter directive, since otherwise the linter would remove the redundant raw values

@calda Thank you for your explanation! I learned something new today! I have fixed accordingly. Please review again. 😄

… external source.

calda · 2023-03-21T16:10:53Z

README.md

  enum UserType: String {
    case owner = "owner"
    case manager = "manager"
    case member = "member"
  }
-  // swiftformat:enable redundantRawValues


The way this directive works is that you:

disable the rule with swiftformat:disable redundantRawValues, so that SwiftFormat doesn't try to modify the enum

write the enum declaration

re-enable the rule with swiftformat:enable redundantRawValues

So each enum needs both the disable and then enable directive. We'll want to keep this one here, and then add the correct swiftformat:disable redundantRawValues and then swiftformat:enable redundantRawValues to the enum Planet: Int declaration below.

@calda Now, I totally understand why swiftformat:disable redundantRawValues and swiftformat:enable redundantRawValueswere there. Thank you so much for your explanation. I truly appreciate your time and effort.

calda · 2023-03-21T16:11:36Z

README.md


+  // RIGHT
+  // swiftformat:enable redundantRawValues


This should be:

Suggested change

// swiftformat:enable redundantRawValues

// swiftformat:disable redundantRawValues

with the corresponding // swiftformat:enable redundantRawValues directive after the closing } of the enum

Thanks for adding this here though!

@calda I have implemented swiftformat:disable redundantRawValues and swiftformat:enable redundantRawValues to enum Planet: Int like you mentioned. Once again, thank you for taking your time to explain everything. I really appreciate you.

@calda By the way, should enum: ErrorType: String and enum: ErrorCode need to be implemented too?

No, since those enums don't have any "redundant" raw values (e.g. case error = "error" would be "redundant" since the compiler generates that automatically)

@calda Sorry to bother you again, but I am confused with "redundant" raw values and usage of swiftformat.

As I have read rules.md of swiftformat and your comment for serveral times, I think

enum ErrorType: String in WRONG is the one that needs to be implemented with swiftformat:disable redundantRawValues and swiftformat:enable redundantRawValues as it needs to be excepted from swiftformat rules.

enum Planet: Int needs to be reverted as before, because it is straightly following Swift's automatic enum values, and it does not contain any redundant raw values.

May I hear back what you think on these two?
Thank you for your time in advance.

Oops! Sorry, you're right -- my mistake. Mind making another PR to fix that?

Thank you for getting back to me! I am on it. Allow me few seconds please! :)

… external source.

Clarify Rule: Use Swift's automatic enum values unless they map to an…

7dd87f5

… external source.

calda approved these changes Mar 21, 2023

View reviewed changes

Clarify Rule: Use Swift's automatic enum values unless they map to an…

8fa105d

… external source.

calda reviewed Mar 21, 2023

View reviewed changes

teddy5518 added 2 commits March 22, 2023 01:21

Clarify Rule: Use Swift's automatic enum values unless they map to an…

d9ad3a6

… external source.

Clarify Rule: Use Swift's automatic enum values unless they map to an…

a752a90

… external source.

calda merged commit 6071ded into airbnb:master Mar 21, 2023

teddy5518 mentioned this pull request Mar 21, 2023

Clarify Rule: Use Swift's automatic enum values unless they map to an external source. #216

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify Rule: Use Swift's automatic enum values unless they map to an external source. #215

Clarify Rule: Use Swift's automatic enum values unless they map to an external source. #215

teddy5518 commented Mar 21, 2023 •

edited

Loading

calda Mar 21, 2023

teddy5518 Mar 21, 2023

calda Mar 21, 2023

teddy5518 Mar 21, 2023

calda Mar 21, 2023

calda Mar 21, 2023

teddy5518 Mar 21, 2023

teddy5518 Mar 21, 2023

calda Mar 21, 2023

teddy5518 Mar 21, 2023 •

edited

Loading

calda Mar 21, 2023

teddy5518 Mar 21, 2023

		/// These are written to a logging service. Explicit values ensure they're consistent across binaries.
		// swiftformat:disable redundantRawValues

	// swiftformat:enable redundantRawValues
	// swiftformat:disable redundantRawValues

Clarify Rule: Use Swift's automatic enum values unless they map to an external source. #215

Clarify Rule: Use Swift's automatic enum values unless they map to an external source. #215

Conversation

teddy5518 commented Mar 21, 2023 • edited Loading

Summary

Reasoning

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teddy5518 Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teddy5518 commented Mar 21, 2023 •

edited

Loading

teddy5518 Mar 21, 2023 •

edited

Loading