Closed wmcginty closed 2 years ago
Thoughts @BottleRocketStudios/team-ios-open-source-w ?
I agree with all of your points. As far as the library/target naming goes, should we just go ahead and create a target for each utility? It might be a bit more verbose to need to refer to things like Instantiation, General (probably needs a better name), and Version separately, but then at least it's clear what you're getting.
I like the idea of separating the targets into different libraries. Having one library for each target sounds like a good approach.
General
. I don't know any better names right now except Common
or Support
.UIPageControl
does make ScrollingPageControl
obsolete, then I'm for removing it.ActiveLabel
public sounds easy enough.ContainerViewController
in favor of iOS-Container
down the road sounds good.ScrollingPageControl
and ContainerViewController
will be removed, then it becomes less complex. Can still decide to move some to their own package in the future.Something else that may be worth doing is creating separate READMEs (e.g. Instantiation.md) for each library and have the main README link to them. The main README is getting massive.
I was messing around with this a bit on #104 and I wanted to post some pictures so we're all on the same page as to when the individual libraries / targets matter.
When you initially add the dependency, you are given the choice of libraries available, and which targets in the project they should be added to.
These same libraries are available to manually add to targets later on.
In order to use the contents of the library (UtiliKitCore
in this case), you instead import the names of the targets contained in each library (GeneralUtilities
and InstantiationUtilities
in this case).
The process of adding packages shown here is one I am familiar with. Seeing 3 different libraries in this example gets me thinking more about the new organization. One way I see it working is having a library for each target as discussed in previous comments. Then there could be a library, UtiliKit
, that houses all targets. I'm not sure how valuable having UtiliKitCore
still is.
I've not worked with integrating Swift packages much, but based on the screenshots of the flow that @wmcginty posted, I'm thinking it might be nice if we can get it so that the name of the target used in the import
is the same as the name of the library/package being added. That way it's a clear 1-1 naming convention and you don't need to know that adding something like a package called UtiliKitCore
means that you need to import
something called GeneralUtilities
.
I have started an implementation of #103 in #104. It looks like doing this effectively will force us to rename some of the library names (it seems likewe can not have files compiled into multiple targets (https://forums.swift.org/t/swiftpm-same-sources-multiple-targets/48810).
Because doing so would effectively force a major version change, I'd like to propose we take that opportunity to do a larger re-organization / cleanup of the 'sub packages' in UtiliKit. Maybe something like):
Library: CoreUtilities Target(s): Instantiation + General + Version
Library: InterfaceUtilities Target(s): ActiveLabel + ScrollingPageControl
Library: ContainerViewController Target(s): Container
Library: TimelessDate Target(s): TimelessDate
Library: Obfuscation Target(s): Obfuscation
A few other notes:
ScrollingPageControl
is currently unable to be imported (it isn't part of theCore
subspec and has no separate subspec. With the changes toUIPageControl
in iOS 14 - it may be time to deprecate / remove anyway?ActiveLabel
is marked asinternal
, so efffectively also unusable.ContainerViewController
. I left it in here as it's used by quite a few projects, but it might be worth considering deprecating / removing in favor of iOS-Container in a new major version.Package.swift
because SwiftPM currently has no way of saying 'this is a dependency for tests only' and theActiveLabelTests
rely onSnapshotTesting
(these tests could also be re-worked to solve that issue).