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

Update Podspec to use RAC 4.2.1 #132

Closed
wants to merge 5 commits into from

Conversation

minuscorp
Copy link

Maybe a version bump and/or a release would be better than applying a
defined commit number.

Maybe a version bump and/or a release would be better than applying a
defined commit number.
@minuscorp
Copy link
Author

I don't know why this is not linting on Travis, as it goes fine in local, I think creating a tag and modifying the target tag for the podspec should fix it.

@@ -18,8 +18,8 @@ Pod::Spec.new do |s|
s.watchos.deployment_target = '2.0'
s.tvos.deployment_target = '9.0'

s.source = { :git => 'https://github.com/neilpa/Rex.git', :tag => s.version }
s.dependency 'ReactiveCocoa', '~> 4.1'
s.source = { :git => 'https://github.com/neilpa/Rex.git', :commit => '265e8e9c9503bc917ee5af5252590ab38eda4bff' }
Copy link
Contributor

@bjarkehs bjarkehs Jul 5, 2016

Choose a reason for hiding this comment

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

No need to change this line. We should simply have @neilpa cut a new version. It is a little impractical we are so heavily tied to to the RAC version.

It fails linting due to:

-> Rex (0.11.0)
    - WARN  | xcodebuild:  ReactiveCocoa/ReactiveCocoa/Objective-C/RACCompoundDisposable.m:85:12: warning: unused variable 'result' [-Wunused-variable]
    - WARN  | xcodebuild:  ReactiveCocoa/ReactiveCocoa/Objective-C/RACCompoundDisposable.m:132:12: warning: unused variable 'result' [-Wunused-variable]
    - WARN  | xcodebuild:  ReactiveCocoa/ReactiveCocoa/Objective-C/RACSerialDisposable.m:63:12: warning: unused variable 'result' [-Wunused-variable]
    - WARN  | xcodebuild:  ReactiveCocoa/ReactiveCocoa/Objective-C/RACSerialDisposable.m:79:12: warning: unused variable 'result' [-Wunused-variable]

I guess there is not really any other way around this than to simply ignore the failed builds so far.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't actually knew that warnings made linting to fail. I might have expected that the CI would execute the --allow-warnings by default.

@bjarkehs
Copy link
Contributor

bjarkehs commented Jul 5, 2016

A dirty fix for us CocoaPods lovers could be to make a 0.10.0a or similar that's tied to a specific commit. But I would prefer going through official releases :)

@minuscorp
Copy link
Author

I'm really waiting for someone to make a new tag in the repo so I can change the podspec, no intention in merge this as it is right now because I understand how dirty my initial solution is ;)

@neilpa
Copy link
Member

neilpa commented Jul 5, 2016

I just added a 0.11.0-beta.1 tag but don't have time currently to cut an official release w/ notes.

@@ -18,7 +18,7 @@ Pod::Spec.new do |s|
s.watchos.deployment_target = '2.0'
s.tvos.deployment_target = '9.0'

s.source = { :git => 'https://github.com/neilpa/Rex.git', :commit => '265e8e9c9503bc917ee5af5252590ab38eda4bff' }
s.source = { :git => 'https://github.com/neilpa/Rex.git', :tag => '0.11.0-beta.1' }
Copy link
Contributor

Choose a reason for hiding this comment

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

:tag => s.version

Copy link
Author

Choose a reason for hiding this comment

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

My bad 😅

@@ -19,7 +19,7 @@ Pod::Spec.new do |s|
s.tvos.deployment_target = '9.0'

s.source = { :git => 'https://github.com/neilpa/Rex.git', :tag => s.version }
Copy link
Member

Choose a reason for hiding this comment

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

You should update this to RACCommunity/Rex now that I've transferred the repo

Copy link
Member

Choose a reason for hiding this comment

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

And the homepage field above too

Copy link
Author

Choose a reason for hiding this comment

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

Of course! Didn't know about it, sorry.

Copy link
Contributor

Choose a reason for hiding this comment

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

@neilpa what about author?

Copy link
Member

Choose a reason for hiding this comment

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

Technically I'm more or less still the author. We could add others since I think that supports a list of authors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't trying to discredit you, just wondering if it should be updated too :)

Copy link
Member

Choose a reason for hiding this comment

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

I know you weren't. I'm just not sure what we would put there instead.

@dmcrodrigues
Copy link
Contributor

Superseded by #141.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants