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

Fix installing Eureka pod #85

Merged
merged 1 commit into from Apr 1, 2019

Conversation

Projects
None yet
6 participants
@armstrongnate
Copy link
Contributor

armstrongnate commented Apr 1, 2019

refs: none
affects: parent
release note: none

I couldn't find a version of Eureka that builds with Swift 3 so we're
pulling in a Swift 4.2 version. I had to tweak the Podfile in order to
build Eureka with Swift 4.2.

Fix installing Eureka pod
refs: none
affects: parent
release note: none

I couldn't find a version of Eureka that builds with Swift 3 so we're
pulling in a Swift 4.2 version. I had to tweak the Podfile in order to
build Eureka with Swift 4.2.
@bootstraponline
Copy link
Member

bootstraponline left a comment

always nice to use stable release versions 😄

config.build_settings['SWIFT_VERSION'] = '3.0'
unless usePackageSwift.include? target.name
config.build_settings['SWIFT_VERSION'] = '3.0'
end

This comment has been minimized.

Copy link
@vanwagonet

vanwagonet Apr 1, 2019

Contributor

I like this approach. I figured we were going to need to do this as we migrated anyway, so it's good to see it look nice in practice.

@@ -102,7 +102,7 @@ open class StudentSettingsViewController : FormViewController {
let sectionTitle = NSLocalizedString("Alert me when:", comment: "Alert Section Header")
form +++
Section() {
$0.header = studentSectionHeaderView()
$0.header = self.studentSectionHeaderView()

This comment has been minimized.

Copy link
@bluwave

bluwave Apr 1, 2019

Contributor

should this be weak or should be no worries here?

This comment has been minimized.

Copy link
@armstrongnate

armstrongnate Apr 1, 2019

Author Contributor

I think we are good. Section does not hold a reference to the initializer block.

This is what it looks like:

    public init(_ initializer: @escaping (Section) -> Void) {
        initializer(self)
    }

Not really sure why they mark is as @escaping.

This comment has been minimized.

Copy link
@armstrongnate

armstrongnate Apr 1, 2019

Author Contributor

Also, we don't hold a reference to the Section. Looks like it's just used to build a view then de-initialize.

@inst-danger

This comment has been minimized.

Copy link
Contributor

inst-danger commented Apr 1, 2019

Warnings
⚠️

This pull request will not generate a release note.

Affected Apps: Parent

Coverage New % Master % Delta
Core 94.51% 94.51% 0%
Student 93.29% 93.29% 0%
Teacher 100% 100% 0%
React Native 92.53% 92.53% 0%

Generated by 🚫 dangerJS

@armstrongnate armstrongnate merged commit d01d9be into master Apr 1, 2019

10 checks passed

Danger :warning: Danger found some issues. Don't worry, everything is fixable.
Details
ci/bitrise/13a8648ad7201c0d/push Passed - Canvas iOS Teacher
Details
ci/bitrise/72a41ea6e1d3f173/pr Passed - Canvas iOS Danger
Details
ci/bitrise/88a3641e29e4745c/push Passed - Canvas iOS Student
Details
ci/bitrise/bd385a53a16abb46/push Passed - Canvas iOS Parent
Details
license/cla Contributor License Agreement is signed.
Details
license/snyk - package.json (Instructure) No manifest changes detected
license/snyk - rn/Teacher/package.json (Instructure) No manifest changes detected
security/snyk - package.json (Instructure) No manifest changes detected
security/snyk - rn/Teacher/package.json (Instructure) No manifest changes detected

@armstrongnate armstrongnate deleted the fix-eureka branch Apr 1, 2019

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.