Skip to content

Conversation

@yim-lee
Copy link
Member

@yim-lee yim-lee commented Aug 27, 2019

Motivation:
1...5 gets reformatted into 1 ... 5.

Modifications:
Add swiftformat rule --ranges no-space.

Result:
swiftformat doesn't insert space around either side of the range operator.

Motivation:
`1...5` gets reformatted into `1 ... 5`.

Modifications:
Add swiftformat rule `--ranges no-space`.

Result:
swiftformat doesn't insert space around either side of the range operator.
Copy link
Member Author

@yim-lee yim-lee left a comment

Choose a reason for hiding this comment

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

@ktoso wdyt?

I prefer this and this is what Google Swift Style Guide suggests:

image

--comments ignore
--ifdef no-indent
--patternlet inline
--ranges no-space
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this.

The rest I put in alphabetical order.

```
/Users/ylee/apple/swift-distributed-actors/Samples/SampleMetrics/main.swift:83:23: error: ambiguous reference to member 'collect'
                print(prom.collect())
                      ^~~~
Prometheus.PrometheusClient:3:17: note: found this candidate
    public func collect(_ succeed: (String) -> ())
                ^
Prometheus.PrometheusClient:4:17: note: found this candidate
    public func collect(into promise: NIO.EventLoopPromise<String>)
                ^
Prometheus.PrometheusClient:5:17: note: found this candidate
    public func collect(_ succeed: (NIO.ByteBuffer) -> ())
                ^
Prometheus.PrometheusClient:6:17: note: found this candidate
    public func collect(into promise: NIO.EventLoopPromise<NIO.ByteBuffer>)
```

Copied from ac32225
@yim-lee
Copy link
Member Author

yim-lee commented Aug 27, 2019

Same build failure as mentioned in #40 (comment).

Copied fix from ac32225 in the same PR.

@yim-lee yim-lee requested a review from ktoso August 27, 2019 19:32
@yim-lee
Copy link
Member Author

yim-lee commented Aug 27, 2019

@tomerd @weissi wdyt? I can submit PR in other repos to make sure we are consistent.

@ktoso
Copy link
Member

ktoso commented Aug 27, 2019

I like this 👍 and evenmore so if it aligns with the google style guide, which supposedly is basically the stdlib guide.

@ktoso
Copy link
Member

ktoso commented Aug 27, 2019

Let's check with the others if we want to ensure we all use the same style or not etc.

Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

As far as I'm concerned this is nice 👍

let's see if others take issue with formatting ¯_(ツ)_/¯

@drexin
Copy link
Member

drexin commented Aug 28, 2019

I prefer it with spaces, especially when using variables. Another thing that doesn't work without spaces is Int.random(in: .min ... .max). Without spaces that doesn't compile.

@ktoso
Copy link
Member

ktoso commented Aug 29, 2019

Hmmm that's a good point... should we keep the current style then? @yim-lee @drexin ?

(the .max argument conviences me)

@ktoso ktoso self-requested a review August 29, 2019 02:05
@yim-lee
Copy link
Member Author

yim-lee commented Aug 29, 2019

Yep, let's not make this change.

@yim-lee yim-lee closed this Aug 29, 2019
@yim-lee yim-lee deleted the range-nospace branch August 29, 2019 02:17
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.

3 participants