Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(297)

Issue 29664569: Favicon: Issue 6245 - SwiftLinted project files (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 1 week ago by a.shephard
Modified:
8 months ago
Reviewers:
d108, dean
Visibility:
Public.

Description

Favicon: Issue 6245 - SwiftLinted project files

Patch Set 1 #

Patch Set 2 : Favicon: Issue 6245 - SwiftLinted project files #

Patch Set 3 : Favicon: Issue 6245 - Swiftlinted project files #

Total comments: 4

Patch Set 4 : Favicon: Issue 6245 - Swiftlinted project files, revised. #

Total comments: 2

Patch Set 5 : Added sorted_imports ignore command to FavIconTests class #

Patch Set 6 : Removed the sorted_imports rule and reversed the import calls to be sorted alphabetically #

Unified diffs Side-by-side diffs Delta from patch set Stats (+348 lines, -339 lines) Patch
M .swiftlint.yml View 1 2 1 chunk +26 lines, -5 lines 0 comments Download
M FavIcon/DetectedIcon.swift View 2 chunks +4 lines, -2 lines 0 comments Download
M FavIcon/FavIcon.swift View 1 2 3 4 18 chunks +56 lines, -46 lines 0 comments Download
M FavIcon/IconExtraction.swift View 1 2 3 7 chunks +216 lines, -236 lines 0 comments Download
M FavIcon/URLRequestOperation.swift View 3 chunks +9 lines, -7 lines 0 comments Download
M FavIcon/Utils.swift View 1 2 3 1 chunk +32 lines, -16 lines 0 comments Download
M FavIconTests/FavIconsTests.swift View 1 2 3 5 4 chunks +4 lines, -8 lines 0 comments Download
M FavIconTests/HTMLDocumentTests.swift View 1 chunk +1 line, -1 line 0 comments Download
M Walkthrough.playground/Contents.swift View 1 1 chunk +0 lines, -18 lines 0 comments Download

Messages

Total messages: 10
a.shephard
9 months, 1 week ago (2018-01-12 11:05:17 UTC) #1
a.shephard
Updated w/ new patchset containing more linting.
9 months, 1 week ago (2018-01-12 16:51:20 UTC) #2
dean
On 2018/01/12 16:51:20, a.shephard wrote: > Updated w/ new patchset containing more linting. LGTM
9 months, 1 week ago (2018-01-13 00:43:57 UTC) #3
d108
https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/FavIcon.swift File FavIcon/FavIcon.swift (right): https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/FavIcon.swift#newcode21 FavIcon/FavIcon.swift:21: #if os(iOS) Swiftlint detects these imports out of order ...
8 months, 1 week ago (2018-02-13 21:48:45 UTC) #4
a.shephard
Changes have been made in this patchset to reflact the proposed changes from d108 in ...
8 months ago (2018-02-16 09:06:44 UTC) #5
d108
https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swift File FavIcon/FavIcon.swift (right): https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swift#newcode21 FavIcon/FavIcon.swift:21: #if os(OSX) Swiftlint is still giving warning: Sorted Imports ...
8 months ago (2018-02-16 23:27:18 UTC) #6
a.shephard
On 2018/02/16 23:27:18, d108 wrote: > https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swift > File FavIcon/FavIcon.swift (right): > > https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swift#newcode21 > ...
8 months ago (2018-02-19 10:31:14 UTC) #7
a.shephard
I've updated the codereview a couple of times with different patchsets. Explained in the iOS ...
8 months ago (2018-02-19 10:46:01 UTC) #8
d108
On 2018/02/19 10:46:01, a.shephard wrote: > I've updated the codereview a couple of times with ...
8 months ago (2018-02-20 01:03:13 UTC) #9
dean
8 months ago (2018-02-20 14:48:50 UTC) #10
On 2018/02/20 01:03:13, d108 wrote:
> On 2018/02/19 10:46:01, a.shephard wrote:
> > I've updated the codereview a couple of times with different patchsets.
> > Explained in the iOS chat, sorted_imports are not raising a warning for me
> > within Xcode 9.2 and Swiftlint 0.25.0.
> > If I add the sorted_imports rule, it produces an error about superfluous
rules
> > in SwiftLint.
> 
> I believe I received the warnings due to a corrupted swiftlint installation.
I'm
> not sure how that happened but I've fixed it by uninstalling and reinstalling
> swiftlint. I may have been running 0.22.0 even though it was reporting 0.25.0.
> 
> So sorry for misreporting the warnings - LGTM for patch set 6.

Latest revision LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5