Remote project support for PBXContainerItemProxy #425

damirdavletov · 2019-05-20T14:43:03Z

Short description 📝

PBXContainerItemProxy can reference objects defined either within the same .xcodeproj or within a remote one.

If the object is within the same .xcodeproj as the proxy then containerPortal property contains PBXProject object and remoteGlobalIDString contains the ID of the object which can be accessed through the normal object pool.
On the other hand if the referenced object is in remote .xcodeproj then containerPortal property contains PBXFileReference of remote .xcodeproj (where the object is located) and remoteGlobalIDString contains the ID of the object which cannot be accessed through the normal object pool so has to accessed as a string.

Currently support for remote project is absent so I'm adding it in this pull request.

Solution 📦

I added containerPortalAsFileReference and remoteGlobalIDString properties to be able to access the values.
Also isContainerPortalFileReference is introduced in order to check if containerPortal is a file reference or not.
Additionally I split the init method onto 2 convenience initialisers, one for local proxy and one for remote.

…ainer-portal-as-file-reference

pepicrft · 2019-05-20T15:48:44Z

Sources/xcodeproj/Objects/Files/PBXContainerItemProxy.swift

    var containerPortalReference: PBXObjectReference

-    /// Returns the project that contains the remote object.
+    /// Returns the project that contains the remote object. If container portal is a remote project this getter will fail. Use isContainerPortalFileReference to check if you can use the getter


Considering that we are in the middle of a major release, I'd take the opportunity to make the API nicer. What about something like:

enum ContainerPortal { case project(PBXProject) case fileReference(PBXFileReference) } // PBXContainerItemProxy class PBXContainerItemProxy { public var containerPortal: ContainerPortal }

What do you think @damirdavletov ?

@pepibumur I agree regarding making the API nicer. My objective was not introduce breaking changes, but if that's not problem I'll try to make it nicer. Thanks for the feedback!

Yeah, we avoid introducing breaking changes but this is a special case because we are working on a major release. Feel free to leverage Swift features to make the API nice :)

pepicrft · 2019-05-20T15:51:14Z

Sources/xcodeproj/Objects/Files/PBXContainerItemProxy.swift

+            return remoteGlobalIDReference?.value
+        }
+        set {
+            remoteGlobalIDReference = newValue.map { PBXObjectReference($0) }


I might be wrong because it's been a long time without playing with this project but I'd say the remoteGlobalIDReference is not necessary. We can just have removeGlobalID of type String.

pepicrft

This is a great idea @damirdavletov. Thanks for bringing this to the project 👏

codecov · 2019-05-21T11:06:46Z

Codecov Report

Merging #425 into master will decrease coverage by 0.12%.
The diff coverage is 51.85%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
- Coverage   79.01%   78.88%   -0.13%     
==========================================
  Files         141      141              
  Lines        6842     6862      +20     
==========================================
+ Hits         5406     5413       +7     
- Misses       1436     1449      +13

Impacted Files	Coverage Δ
...codeproj/Objects/Files/PBXContainerItemProxy.swift	`66.12% <51.85%> (-14.83%)`	⬇️

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 07f1ebd...36dfe1e. Read the comment docs.

codecov · 2019-05-21T11:06:46Z

Codecov Report

Merging #425 into master will increase coverage by 0.06%.
The diff coverage is 82.05%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   79.01%   79.07%   +0.06%     
==========================================
  Files         141      141              
  Lines        6842     6859      +17     
==========================================
+ Hits         5406     5424      +18     
+ Misses       1436     1435       -1

Impacted Files	Coverage Δ
...codeproj/Objects/Sourcery/Equality.generated.swift	`93.93% <100%> (ø)`	⬆️
...ojTests/Objects/Targets/PBXNativeTargetTests.swift	`100% <100%> (ø)`	⬆️
...es/xcodeproj/Objects/Targets/PBXNativeTarget.swift	`92.1% <100%> (ø)`	⬆️
Sources/xcodeproj/Utils/ReferenceGenerator.swift	`85.48% <50%> (ø)`	⬆️
...codeproj/Objects/Files/PBXContainerItemProxy.swift	`88.13% <81.81%> (+7.18%)`	⬆️

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 07f1ebd...65f6738. Read the comment docs.

damirdavletov · 2019-05-21T11:13:54Z

@pepibumur Thanks for the improvement suggestions! Please have a look at the amended implementation as per your comments.

pepicrft · 2019-05-22T07:26:30Z

Sources/xcodeproj/Objects/Files/PBXContainerItemProxy.swift

@@ -1,5 +1,11 @@
 import Foundation

+public enum ContainerPortal {


I'd move this one into PBXContainerItemProxy

@pepibumur this is done

pepicrft · 2019-05-22T07:27:19Z

Only one more thing @damirdavletov, could you update the CHANGELOG? Make sure that entry that you add states that the change is a breaking change.

…it Equatable

codecov-io · 2019-05-22T09:49:54Z

Codecov Report

Merging #425 into master will increase coverage by 0.06%.
The diff coverage is 82.05%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   79.01%   79.07%   +0.06%     
==========================================
  Files         141      141              
  Lines        6842     6859      +17     
==========================================
+ Hits         5406     5424      +18     
+ Misses       1436     1435       -1

Impacted Files	Coverage Δ
...codeproj/Objects/Sourcery/Equality.generated.swift	`93.93% <100%> (ø)`	⬆️
...ojTests/Objects/Targets/PBXNativeTargetTests.swift	`100% <100%> (ø)`	⬆️
...es/xcodeproj/Objects/Targets/PBXNativeTarget.swift	`92.1% <100%> (ø)`	⬆️
Sources/xcodeproj/Utils/ReferenceGenerator.swift	`85.48% <50%> (ø)`	⬆️
...codeproj/Objects/Files/PBXContainerItemProxy.swift	`88.13% <81.81%> (+7.18%)`	⬆️

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 07f1ebd...cd9db92. Read the comment docs.

codecov-io · 2019-05-22T09:49:54Z

Codecov Report

Merging #425 into master will increase coverage by 0.06%.
The diff coverage is 82.05%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   79.01%   79.07%   +0.06%     
==========================================
  Files         141      141              
  Lines        6842     6859      +17     
==========================================
+ Hits         5406     5424      +18     
+ Misses       1436     1435       -1

Impacted Files	Coverage Δ
...codeproj/Objects/Sourcery/Equality.generated.swift	`93.93% <100%> (ø)`	⬆️
...ojTests/Objects/Targets/PBXNativeTargetTests.swift	`100% <100%> (ø)`	⬆️
...es/xcodeproj/Objects/Targets/PBXNativeTarget.swift	`92.1% <100%> (ø)`	⬆️
Sources/xcodeproj/Utils/ReferenceGenerator.swift	`85.48% <50%> (ø)`	⬆️
...codeproj/Objects/Files/PBXContainerItemProxy.swift	`88.13% <81.81%> (+7.18%)`	⬆️

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 07f1ebd...cd9db92. Read the comment docs.

damirdavletov · 2019-05-22T09:52:30Z

Only one more thing @damirdavletov, could you update the CHANGELOG? Make sure that entry that you add states that the change is a breaking change.

This was already there. Let me know if you need anything else done. Thanks!

yonaskolb · 2019-06-07T20:21:42Z

I think this may have broken things. Code outside this project can't get access to the uuid of an object, so there isn't a way to access a value to pass into remoteGlobalIDString

yonaskolb · 2019-06-07T20:25:17Z

In addition as the uuid may be generated after everything is created, it's not possible to access it anyway. This may need to become an enum instead of a string

enum RemoteGlobalID {
	case object(PBXObject)
	case uuid(String)
}

pepicrft · 2019-06-07T21:11:28Z

Would you mind opening an issue with that @yonaskolb?

damirdavletov added 3 commits May 20, 2019 15:00

Remote project support for PBXContainerItemProxy

107c919

Merge branch 'master' of https://github.com/badoo/xcodeproj into cont…

15f63d8

…ainer-portal-as-file-reference

Changelog update

5a92cf9

pepicrft reviewed May 20, 2019

View reviewed changes

Introduced ContainerPortal enum for PBXContainerItemProxy

36dfe1e

Updated change log

65f6738

pepicrft reviewed May 22, 2019

View reviewed changes

Moved ContainerPortal enum into PBXContainerItemProxy class and made …

cd9db92

…it Equatable

pepicrft approved these changes May 22, 2019

View reviewed changes

pepicrft merged commit a47ec78 into tuist:master May 22, 2019

yonaskolb mentioned this pull request Jun 10, 2019

Fix PBXContainerItemProxy.remoteGlobalID #445

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remote project support for PBXContainerItemProxy #425

Remote project support for PBXContainerItemProxy #425

damirdavletov commented May 20, 2019 •

edited

Loading

pepicrft May 20, 2019

damirdavletov May 20, 2019

pepicrft May 21, 2019

pepicrft May 20, 2019

pepicrft left a comment

codecov bot commented May 21, 2019

codecov bot commented May 21, 2019 •

edited

Loading

damirdavletov commented May 21, 2019

pepicrft May 22, 2019

damirdavletov May 22, 2019

pepicrft commented May 22, 2019

codecov-io commented May 22, 2019

codecov-io commented May 22, 2019

damirdavletov commented May 22, 2019

yonaskolb commented Jun 7, 2019

yonaskolb commented Jun 7, 2019 •

edited

Loading

pepicrft commented Jun 7, 2019

		@@ -1,5 +1,11 @@
		import Foundation

		public enum ContainerPortal {

Remote project support for PBXContainerItemProxy #425

Remote project support for PBXContainerItemProxy #425

Conversation

damirdavletov commented May 20, 2019 • edited Loading

Short description 📝

Solution 📦

pepicrft May 20, 2019

Choose a reason for hiding this comment

damirdavletov May 20, 2019

Choose a reason for hiding this comment

pepicrft May 21, 2019

Choose a reason for hiding this comment

pepicrft May 20, 2019

Choose a reason for hiding this comment

pepicrft left a comment

Choose a reason for hiding this comment

codecov bot commented May 21, 2019

Codecov Report

codecov bot commented May 21, 2019 • edited Loading

Codecov Report

damirdavletov commented May 21, 2019

pepicrft May 22, 2019

Choose a reason for hiding this comment

damirdavletov May 22, 2019

Choose a reason for hiding this comment

pepicrft commented May 22, 2019

codecov-io commented May 22, 2019

Codecov Report

codecov-io commented May 22, 2019

Codecov Report

damirdavletov commented May 22, 2019

yonaskolb commented Jun 7, 2019

yonaskolb commented Jun 7, 2019 • edited Loading

pepicrft commented Jun 7, 2019

damirdavletov commented May 20, 2019 •

edited

Loading

codecov bot commented May 21, 2019 •

edited

Loading

yonaskolb commented Jun 7, 2019 •

edited

Loading