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

Consolidate into package #186

Merged
merged 1 commit into from
Jan 25, 2022
Merged

Consolidate into package #186

merged 1 commit into from
Jan 25, 2022

Conversation

kmcbride
Copy link
Collaborator

Recreating this PR for a few reasons:

  • Simplifies maintenance
    • Compatibility and releases are easier to manage (as illustrated by our oft-outdated CocoaPods specs)
  • Removes excess infrastructure
    • Projects, Schemes, and Configs are no longer needed
  • Improves the development workflow
    • Building and testing for platforms besides iOS previously needed bootstrap script and project configuration adjustments
  • Helps with other tooling
    • Declaring CasePaths as an explicit dependency would be overly cumbersome if also continuing to support CocoaPods, but keeping it external is preferable because it allows us to more easily integrate and attribute it via our tooling

@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #186 (5952412) into master (714b634) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
+ Coverage   95.34%   95.37%   +0.03%     
==========================================
  Files          43       42       -1     
  Lines        1439     1427      -12     
==========================================
- Hits         1372     1361      -11     
+ Misses         67       66       -1     
Flag Coverage Δ
ios ?
macspm 95.37% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
MobiusCore/Source/EffectHandlers/EnumRoute.swift 75.00% <100.00%> (ø)
...rowableAssertion/Source/MobiusThrowableAssertion.m

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 714b634...5952412. Read the comment docs.

Copy link
Contributor

@rastersize rastersize left a comment

Choose a reason for hiding this comment

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

This makes sense to me.

I do wonder how many, if any?, folks use CocoaPods to integrate Mobius? I guess anyone that need it could either create their own spec or fork?

@kmcbride
Copy link
Collaborator Author

kmcbride commented Jan 25, 2022

I do wonder how many, if any?, folks use CocoaPods to integrate Mobius?

It's hard to say, but my guess would be very few. I tried searching across GitHub and practically nothing comes up. There's also a button in the sidebar of the CocoaPods page that doesn't show any results for apps using it (but maybe that requires additional configuration?).

I guess anyone that need it could either create their own spec or fork?

Yes, one should fork and create a Podspec if they need continued CocoaPods support. This is part of the reason why I referred to importing CasePaths while supporting CocoaPods as cumbersome 😬

@rastersize
Copy link
Contributor

@kmcbride Sounds like we should go ahead and merge this then 👍

@kmcbride kmcbride merged commit c2f958f into spotify:master Jan 25, 2022
@kmcbride kmcbride deleted the spm branch January 25, 2022 20:37
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.

2 participants