-
-
Notifications
You must be signed in to change notification settings - Fork 159
Resolves #92, Identifier Name Violation Issues after updating to Xcode 9.3 #93
Conversation
@@ -32,6 +32,7 @@ class SettingsTableViewController: UITableViewController, MFMailComposeViewContr | |||
} | |||
|
|||
override func tableView(_ tableView: UITableView, titleForHeaderInSection section: Int) -> String? { | |||
// swiftlint:disable:next identifier_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, instead of disabling swiftlint here, the 'Settings' variable should be 'settings' variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed variable Settings to settings
I had two solution for this -
Like, there are many warnings due to 'vc' variable, referring to ViewControllers. We can rename them to SummaryFormTableVC or NutritionTableFormVC etc What do you propose @antsangrigoli @roger-tan ? |
Sources/Extensions/UIColor.swift
Outdated
@@ -19,10 +19,12 @@ extension UIColor { | |||
|
|||
var rgb: UInt32 = 0 | |||
|
|||
// swiftlint:disable identifier_name | |||
var r: CGFloat = 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'm not a huge fan of abbreviation. Instead to disable the error. We should just rename r as red, green as green, b as blue and a as alpha. This will make the code understandable without guessing each abbreviation what does it means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Renamed variables r to red, g to green, b to blue, a to alpha in UIColor.swift
@snuff4 @roger-tan
The only other warnings I can see about an Identifier Name Violation are for the variables "id" and "vc".
As snuff4 mentioned, I'm going to rename each "vc" variable to its specific view controller name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed each "vc" variable to its specific view controller name
I concur. I prefer large variable names as well, so it is clear what a variable represents. By the way, if I have an optional, which I unwrap with |
@@ -31,28 +31,28 @@ class UserViewController: UIViewController, DataManagerClient { | |||
showProductsPendingUpload() | |||
} | |||
|
|||
vc = childNavigationController | |||
UIVC = childNavigationController | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change it to childNavigationVC? And we need to make sure every function of the app is working, after the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed "UIVC" to childNavigationVC
@antsangrigoli can you ensure that everything is working after renaming? |
@snuff4 yes everything seems to work fine. |
@antsangrigoli I saw that too. There's a issue related to this, too. |
@snuff4 Just read up on it. I could add the following parameters below to the .swiftlint.yml file and this will solve the issue. identifier_name: |
poke @roger-tan For this I think we can exclude for this. We need to finish with this PR asap, because without it, it's not working on 9.3 |
@@ -29,8 +29,8 @@ extension UIViewController { | |||
} | |||
|
|||
func openUrlInApp(_ url: URL, showAlert: Bool? = nil) { | |||
let vc = SFSafariViewController(url: url, entersReaderIfAvailable: false) | |||
present(vc, animated: true) | |||
let SFSafariVC = SFSafariViewController(url: url, entersReaderIfAvailable: false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally for local context. I would just call controller but a variable name cannot with an uppercase letter. We should keep the camel case for a name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I should rename the variable SFSafariVC to sfSafariVC ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep consistence inside the projet. Just sfSafariVC.
For id. It's ok for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Renamed SFSafariVC to sfSafariVC
- Added parameter in .swiftlint.yml file to exclude variable name "id"
@antsangrigoli Thank you for your contributions into OpenFoodFacts. Your Pull Request is now merged . |
PR Description
This is in response to issue #92. When building the project in Xcode 9.3, I was getting 9 Identifier Name Violations errors. I've fixed these errors by adding // swiftlint:disable:next identifier_name as you can see in the screenshots below. Project can now be built and ran in Xcode 9.3.
Type of Changes
Screenshots
Before
After
Checklist
Make sure you've done all the following (Put an
x
in the boxes that apply.)