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.

Sign up for GitHub

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

Jump to bottom

Add carthage 0.30.1 static framework support #132

Merged
merged 3 commits into from
Jul 30, 2018
Merged

Add carthage 0.30.1 static framework support #132

merged 3 commits into from
Jul 30, 2018

Conversation

tmspzz
Copy link
Owner

@tmspzz tmspzz commented Jul 8, 2018 •
edited
Loading

@hlintBot
Copy link

hlintBot commented Jul 8, 2018 •
edited
Loading

2 Warnings
⚠️ PR is classed as Work in Progress
⚠️ Big PR

Generated by 🚫 Danger

hlintBot
hlintBot reviewed Jul 9, 2018
View reviewed changes
src/Caches/S3/Downloading.hs
@@ -174,15 +175,15 @@ getAndUnzipBcsymbolmapsFromS3' :: S3.BucketName -- ^ The cache definition
-> ExceptT DWARFOperationError (ReaderT (AWS.Env, CachePrefix, Bool) IO) ()
getAndUnzipBcsymbolmapsFromS3' lCacheDir
reverseRomeMap
fVersion@(FrameworkVersion f@(FrameworkName fwn) _)
fVersion@(FrameworkVersion f@(Framework fwn fwt) _)
platform = do

dwarfUUIDs <- withExceptT (const ErrorGettingDwarfUUIDs) $ dwarfUUIDsFrom (frameworkDirectory </> fwn)
eitherDwarfUUIDsOrSucces <- forM dwarfUUIDs
(\dwarfUUID ->
lift $ runExceptT (withExceptT
(\e -> (dwarfUUID, e)) $
Copy link

@hlintBot hlintBot Jul 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Found Use tuple-section

1\ e -> (dwarfUUID, e)

Why Not

1(dwarfUUID,)

hlintBot
hlintBot reviewed Jul 10, 2018
View reviewed changes
src/Caches/Local/Downloading.hs
@@ -167,7 +167,7 @@ getAndUnzipBcsymbolmapsFromLocalCache lCacheDir
dwarfUUIDs
where
frameworkNameWithFrameworkExtension = appendFrameworkExtensionTo f
Copy link

@hlintBot hlintBot Jul 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Found Reduce duplication

1frameworkNameWithFrameworkExtension = appendFrameworkExtensionTo f
2platformBuildDirectory
3  = carthageArtifactsBuildDirectoryForPlatform platform f
4frameworkDirectory
5  = platformBuildDirectory </> frameworkNameWithFrameworkExtension
6

Why Not

1Combine with src/Caches/Local/Downloading.hs:199:5

hlintBot
hlintBot reviewed Jul 10, 2018
View reviewed changes
src/Lib.hs
@@ -474,7 +474,7 @@ uploadFrameworkAndArtifactsToCaches s3BucketName
where

frameworkNameWithFrameworkExtension = appendFrameworkExtensionTo f
Copy link

@hlintBot hlintBot Jul 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Found Reduce duplication

 1frameworkNameWithFrameworkExtension = appendFrameworkExtensionTo f
 2platformBuildDirectory
 3  = carthageArtifactsBuildDirectoryForPlatform platform f
 4frameworkDirectory
 5  = platformBuildDirectory </> frameworkNameWithFrameworkExtension
 6dSYMNameWithDSYMExtension
 7  = frameworkNameWithFrameworkExtension <> ".dSYM"
 8dSYMdirectory
 9  = platformBuildDirectory </> dSYMNameWithDSYMExtension
10bcSybolMapPath d = platformBuildDirectory </> bcsymbolmapNameFrom d
11

Why Not

1Combine with src/Lib.hs:538:5

@tmspzz tmspzz force-pushed the feature/carthage-0.30.1-static branch 2 times, most recently from 9ca7f77 to c5f860e Compare July 10, 2018 11:55
@tmspzz tmspzz force-pushed the feature/carthage-0.30.1-static branch from b233653 to 4e547b5 Compare July 30, 2018 13:55
@tmspzz tmspzz force-pushed the feature/carthage-0.30.1-static branch from 4e547b5 to b624647 Compare July 30, 2018 16:11
@tmspzz tmspzz changed the title [WIP] Add carthage 0.30.1 static framework support Add carthage 0.30.1 static framework support Jul 30, 2018
@tmspzz tmspzz merged commit faed9ce into master Jul 30, 2018
@tmspzz tmspzz deleted the feature/carthage-0.30.1-static branch July 30, 2018 16:12
@thii
Copy link
Contributor

thii commented Jul 30, 2018

@blender Could you release this version to CocoaPods?

@tmspzz
Copy link
Owner Author

tmspzz commented Jul 30, 2018

@thii I didn't make a full release yet. I'll release everywhere once I am done. If you don't need static framework support this version probably won't change much for you.

If you do need static framework support see:https://github.com/blender/Rome#static-frameworks

@thii
Copy link
Contributor

thii commented Jul 30, 2018

@blender Yes, we do need static framework support. Does the current version work with the new static/ prefix in Romefile?

@tmspzz
Copy link
Owner Author

tmspzz commented Jul 31, 2018 •
edited
Loading

@thii 0.16.0.46 is out on Cocoapods. 0.16.x.x support static/, =< 0.15.x.x don't

@tmspzz
Copy link
Owner Author

tmspzz commented Jul 31, 2018

@thii is cocapods working? I don't see it updated on the website.

This is what I got when running the command, seemed successful to me.

$ pod trunk push Rome.podspec --allow-warnings
Updating spec repo `master`
Validating podspec
 -> Rome (0.16.0.46)
    - WARN  | license: Unable to find a license file

Updating spec repo `master`

--------------------------------------------------------------------------------
 🎉  Congrats

 🚀  Rome (0.16.0.46) successfully published
 📅  July 31st, 08:21
 🌎  https://cocoapods.org/pods/Rome
 👍  Tell your friends!
--------------------------------------------------------------------------------

@thii
Copy link
Contributor

thii commented Jul 31, 2018

@blender No, it’s not. You need to update the version in podspec and do another pod trunk push.

@tmspzz
Copy link
Owner Author

tmspzz commented Jul 31, 2018

On trunk the podspec should be correct. I now pushed the change to the repo too. How can I see the podspec in trunk?

@tmspzz
Copy link
Owner Author

tmspzz commented Jul 31, 2018

@thii https://github.com/CocoaPods/Specs/blob/f0ebfb0f8adb78f7c9a758d6d5f4f11571eea0d1/Specs/1/f/4/Rome/0.16.0.46/Rome.podspec.json

@thii
Copy link
Contributor

thii commented Jul 31, 2018

Oh sorry my local CocoaPods/Specs didn't get updated. I was able to update Rome to 0.16.0.46 now. No idea why it still hasn't get updated on the CocoaPods website.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hlintBot hlintBot hlintBot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tmspzz @hlintBot @thii