T 296 #347

Merged
merged 8 commits into from Jun 25, 2012

Projects

None yet

3 participants

@fabiopelosin
Member

The pull includes a small refactoring to make clear which methods returns relative paths.

It needs testing and the and the relative specs.

@fabiopelosin
Member

The patch is ready and was tested manually with the Chameleon.podspec.

The last commit introduces a minor change in the behavior. Now the header files of a pod are automatically sandboxed in Headers/#{pod.name}. This implies that now Specification#header_dir returns an empty Pathname instead of the name of the pod. This was done for two reasons:

  • it is cleaner
  • it allows always respect the namespaces.

Regarding the last point, in the case of Chameleon the files were stored in Headers/Chameleon/UIKit but the namespace was not respected because Headers/Chameleon was not present in the search paths (as Chameleon doesn't have files at the top level). With this approach instead in the subspecs it is sufficient to specify the namespace (i.e.. UIKit instead of Chameleon/UIKit) and it will respected.

@fabiopelosin
Member

Notes:

  • I've tested the header sandboxing with SSZipArchive and it doesn't break it.
  • All the specs of LocalPod have been enabled (only 4 specs disabled).
  • The Chameleon spec breaks 0.6.0.rc2

@alloy The pull is ready, do you have any objection about it?

@alloy
Member
alloy commented Jun 21, 2012

Looks very good! I even see you figured out how to name dynamically defined methods in the docs in a better way than I knew :D (@!method foo)

The Chameleon spec breaks 0.6.0.rc2

Did you mean that with 0.6.0.rc2 the Chameleon spec breaks and this fixes it?

If so, then please go ahead and merge.

@alloy
Member
alloy commented Jun 21, 2012

Also, does this also fix #296?

@fabiopelosin
Member

Also, does this also fix #296?

It was supposed to fix only that, but I like to mix things :-)

Looks very good! I even see you figured out how to name dynamically defined methods in the docs in a better way than I knew :D (@!method foo)

Yard is very cool! Much better than the solutions that I was trying to use.

Did you mean that with 0.6.0.rc2 the Chameleon spec breaks and this fixes it?

For sure the Chameleon spec could not work after specification refactor. I meant that as we set the header_dir in subspecs CocoaPods v0.6.0.rc2 will raise (as it didn't support it). For that reason the Specs branch increases the minimum version to 0.6.0.rc3.

Btw, was Chameleon spec working with a previous version of CocaPods (I it couldn't so I've updated the commit).


On a related note, in Chameleon some warnings are raise because the Pods.xcodeproj enable some warnings (like the integer conversion one) which are no the OS default. Why?

@alloy

This can't be working can it? The list from select, which is a copy of files, is never saved.

Member

It wasn't thats why the next commit is called [LocalPod] Fix #all_specs_public_header_files. :-)

Member

Doh! I didn't look at the commits in the right direction.

@alloy
Member
alloy commented Jun 21, 2012

Btw, was Chameleon spec working with a previous version of CocaPods (I it couldn't so I've updated the commit).

I think it was…

On a related note, in Chameleon some warnings are raise because the Pods.xcodeproj enable some warnings (like the integer conversion one) which are no the OS default. Why?

That’s a bug then. We should mirror what the default is.

@fabiopelosin
Member

Btw, was Chameleon spec working with a previous version of CocaPods (I it couldn't so I've updated the commit).

I think it was…

I've checked it in Specs:master and it doesn't have header mappings so I guess it couldn't. @urbanappetite @xslim can you confirm?


@alloy Can I merge anyway?

@xslim
Contributor
xslim commented Jun 22, 2012

Sorry, I can't test Chameleon now, I'm on busy different project (

@fabiopelosin
Member

@xslim I wasn't very clear, my question was whether you recall if it was working when you authored the podspec.

@xslim
Contributor
xslim commented Jun 22, 2012

@irrationalfab As I remember it was working )
with the 0.6.0.rc2 (from gem install --pre) it is seems also working...

@fabiopelosin
Member

@xslim Thanks for the quick reply.

@alloy
Member
alloy commented Jun 22, 2012

:shipit:

Go ahead :)

@fabiopelosin fabiopelosin merged commit 16a324a into develop Jun 25, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment