Update to Xcode 9.3 #86
Conversation
.ruby-version
Outdated
| @@ -1 +1 @@ | |||
| ruby-2.4.2 | |||
| 2.4.4 | |||
There was a problem hiding this comment.
What's the version bundled with high Sierra again? We should try to keep the same version of ruby as the one installed by default on the latest macOS, especially for people not familiar with ruby let alone rvm or rbenv or similar tools
There was a problem hiding this comment.
I'm just using what's available on CI:
https://circle-macos-docs.s3.amazonaws.com/image-manifest/build-405/index.html
If people don't have rvm or rbenv, then they won't even notice this change.
There was a problem hiding this comment.
The problem here is that if we later modify our ruby scripts (like Rakefiles) and happen to use methods that are only available in 2.4.4 but not 2.4.2, then CI will pass (and locally on your machine it will probably pass too as you do have rvm) but on any other contributor machine it won't work if they don't have rvm
There was a problem hiding this comment.
From random blog posts I think High Sierra came with ruby 2.3.0?
But like I said, we have to use ruby versions as they are on CI. And I think I had to use ruby 2.4 at some point otherwise some gems kept breaking for me? (during upgrade to CircleCI 2.0)
There was a problem hiding this comment.
Right, I think the default (system) ruby version is 2.0.0, but then I kept hitting this error:
https://circleci.com/gh/SwiftGen/SwiftGen/453
There was a problem hiding this comment.
it's probably worth setting it to 2.3.0 then 😉
There was a problem hiding this comment.
(which is high Sierra's default, which is the min OS required to use Xcode 9 anyway? which is the min Xcode required to build SwiftGen)
Sources/Filters+Strings.swift
Outdated
| @@ -194,7 +194,7 @@ extension Filters.Strings { | |||
| /// - arguments: the arguments to the function; expecting zero | |||
| /// - Returns: the string with first letter being uppercased | |||
| /// - Throws: FilterError.invalidInputType if the value parameter isn't a string | |||
| private static func upperFirstLetter(_ value: String) -> String { | |||
| internal static func upperFirstLetter(_ value: String) -> String { | |||
There was a problem hiding this comment.
The code style on the rest of the code base seem to be not mentioning access control if it's internal (the default); better be consistent here then?
There was a problem hiding this comment.
This is needed to access the method from the SwiftIdentifier bit of code.
Note that, even though there is a change to use
compactMap, this is only in the tests, so it isn't a breaking change for our users.