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 for Carthage to Build #28

Closed
wants to merge 3 commits into from
Closed

Fix for Carthage to Build #28

wants to merge 3 commits into from

Conversation

eSpecialized
Copy link
Contributor

@eSpecialized eSpecialized commented Jun 13, 2019

We found that tests were interfering with SQift builds using carthage, a small change allows them to build, and Xcode to test each Scheme

It was found that tests were interfering with SQift builds using carthage, a small change allows them to build, and Xcode to test each Scheme.
@eSpecialized eSpecialized added this to the 5.0.1 milestone Jun 13, 2019
@eSpecialized eSpecialized requested a review from cnoon June 13, 2019 16:52
@eSpecialized eSpecialized self-assigned this Jun 13, 2019
Copy link
Member

@jimmyeisenhauer jimmyeisenhauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think my approval does anything....

Copy link
Member

@cnoon cnoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small nits. Looks good overall @eSpecialized. If you have time, let's get this pushed out today!

CHANGELOG.md Outdated
@@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.

#### 5.x Releases

- `5.0.x` Releases - [5.0.0](#500)
- `5.0.x` Releases - [5.0.0](#500)||[5.0.1](#501)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[5.0.0](#500)||[5.0.1](#501) -> [5.0.0](#500) | [5.0.1](#501). It's just for markdown formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

CHANGELOG.md Outdated
@@ -41,6 +41,17 @@ All notable changes to this project will be documented in this file.
- `0.2.x` Releases - [0.2.0](#020)
- `0.1.x` Releases - [0.1.0](#010)

## [5.0.1](https://github.com/Nike-Inc/SQift/releases/tag/5.0.1)

Release on 2019-06-13. All issues associated with this milestone can be found using this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release on 2019-06-13. All -> Released on 2019-06-27. All.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not correct. Should be Released and there's an extra space between . All.

CHANGELOG.md Outdated

#### Updated

- Fixes Carthage build issues with failing tests. Updates to Scheme's allows carthage to do the build.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about?

- Fixed issue where Carthage was unable to build SQift due to schemes requiring test suite compilation.
  - Fixed by [William]...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not quite right. Both should start with Fixed and typo in was. Second line is indented two spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

@cnoon cnoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back to you @eSpecialized.

Found the CI’s failing on Travis, so I re-added to the tests in the scheme to get CI working.
cnoon added a commit that referenced this pull request Oct 25, 2019
It was found that tests were interfering with SQift builds using carthage, a small change allows them to build, and Xcode to test each Scheme.
cnoon pushed a commit that referenced this pull request Oct 25, 2019
It was found that tests were interfering with SQift builds using carthage, a small change allows them to build, and Xcode to test each Scheme.
@cnoon
Copy link
Member

cnoon commented Oct 25, 2019

Hey @eSpecialized, I've just merged a modified version of these changes in f88de59. They'll go out shortly as part of the 5.0.1 release. Thank again for putting this together! 🍻

@cnoon cnoon closed this Oct 25, 2019
@cnoon cnoon deleted the carthage-fixes branch October 25, 2019 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants