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

Removed self. when not needed #579

Merged
merged 4 commits into from Oct 10, 2018

Conversation

Projects
None yet
4 participants
@staffler-xyz
Copy link
Contributor

commented Oct 7, 2018

Removed "self." calls where not needed. According to most code style guides (for example https://github.com/raywenderlich/swift-style-guide#use-of-self) they should be avoided where possible. What is your opinion about this?

I didn't add a changelog because it's only a style change (or should I?).

Checklist

  • [x ] I checked the Contributing Guidelines before creating this request.
  • New extensions are written in Swift 4.
  • New extensions support iOS 8.0+ / tvOS 9.0+ / macOS 10.10+ / watchOS 2.0+.
  • I have added tests for new extensions, and they passed.
  • All extensions have a clear comments explaining their functionality, all parameters and return type in English.
  • All extensions are declared as public.
  • I have added a changelog entry describing my changes.
@SwifterSwiftBot

This comment has been minimized.

Copy link

commented Oct 7, 2018

2 Warnings
⚠️ Consider adding tests for new extensions or updating existing tests for a modified SwifterSwift extension
⚠️ The source files have been modified. Please consider adding a CHANGELOG entry if necessary.
4 Messages
📖 Thank you for submitting a pull request to SwifterSwift. The team will review your submission as soon as possible.
📖 iOS: Executed 527 tests, with 0 failures (0 unexpected) in 8.315 (12.921) seconds
📖 tvOS: Executed 496 tests, with 0 failures (0 unexpected) in 3.696 (5.084) seconds
📖 macOS: Executed 361 tests, with 0 failures (0 unexpected) in 1.009 (1.281) seconds

Generated by 🚫 Danger

@codecov

This comment has been minimized.

Copy link

commented Oct 7, 2018

Codecov Report

Merging #579 into master will not change coverage.
The diff coverage is 82.35%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #579   +/-   ##
=======================================
  Coverage   91.23%   91.23%           
=======================================
  Files          70       70           
  Lines        2761     2761           
=======================================
  Hits         2519     2519           
  Misses        242      242
Flag Coverage Δ
#ios 93.43% <100%> (ø) ⬆️
#osx 81.44% <60%> (ø) ⬆️
#tvos 91.86% <100%> (ø) ⬆️
Impacted Files Coverage Δ
Sources/Extensions/AppKit/NSImageExtensions.swift 0% <0%> (ø) ⬆️
Sources/Extensions/AppKit/NSViewExtensions.swift 0% <0%> (ø) ⬆️
Sources/Extensions/UIKit/UIViewExtensions.swift 74.48% <100%> (ø) ⬆️
...tensions/UIKit/UIGestureRecognizerExtensions.swift 100% <100%> (ø) ⬆️
Sources/Extensions/SwiftStdlib/IntExtensions.swift 100% <100%> (ø) ⬆️
...es/Extensions/SwiftStdlib/OptionalExtensions.swift 100% <100%> (ø) ⬆️
...urces/Extensions/UIKit/UITextFieldExtensions.swift 100% <100%> (ø) ⬆️
Sources/Extensions/UIKit/UIButtonExtensions.swift 100% <100%> (ø) ⬆️
...rces/Extensions/SwiftStdlib/StringExtensions.swift 99.42% <100%> (ø) ⬆️
.../Extensions/UIKit/UIViewControllerExtensions.swift 97.29% <100%> (ø) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffe1622...afdb730. Read the comment docs.

@@ -132,7 +132,7 @@ extension NSView {
///
/// - Parameter subviews: array of subviews to add to self.
public func addSubviews(_ subviews: [NSView]) {
subviews.forEach({self.addSubview($0)})
subviews.forEach({addSubview($0)})

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Oct 7, 2018

Member

I know is not the purpose of the PR but can you also fix here to use trailing syntax and adding space between like subviews.forEach { addSubview($0) }? :))

@@ -280,7 +280,7 @@ public extension UIView {
///
/// - Parameter subviews: array of subviews to add to self.
public func addSubviews(_ subviews: [UIView]) {
subviews.forEach({ self.addSubview($0) })
subviews.forEach({ addSubview($0) })

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Oct 7, 2018

Member

Trailing syntax here too :))

self.leftViewMode = UITextField.ViewMode.always
leftView = imageView
leftView?.frame.size = CGSize(width: image.size.width + padding, height: image.size.height)
leftViewMode = UITextField.ViewMode.always

This comment has been minimized.

Copy link
@guykogus

guykogus Oct 8, 2018

Contributor

Can this be changed to leftViewMode = .always?

@guykogus

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2018

Very happy to see this! Good stuff :) Just a few comments left here for you.

@staffler-xyz

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2018

sure, changes from comments added :)

guykogus and others added some commits Oct 9, 2018

@LucianoPAlmeida LucianoPAlmeida merged commit 6ff84b4 into SwifterSwift:master Oct 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.