-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Introduce modularization #5
base: main
Are you sure you want to change the base?
Conversation
Woah that looks really clean, will definitely consider switching to that at some point! But yeah it should be a separate PR.
The examples and associated tests are currently acting as tests for macro toolkit, so to me it makes sense for them to be part of the same package still (it's nice to just run
In your eyes what are the main benefits of allowing the features to be imported separately? I can see why DiagnosticBuilder could make sense to be a separate target, but calling the rest MacroToolkitTypes doesn't really make sense to me. To me that sounds like it's just supporting/helper types, but it also includes extensions, destructuring, and other features. The main reason I'm hesitant to modularise too much is that in Swift it's often tricky to know which import a type came from, because you just have to memorise it or have a really solid distinction between the different targets being imported. For example, I often find myself importing unnecessary SwiftSyntax targets because I can't tell which ones are being used and which ones aren't. And I often have to just guess which one to import to import the type that the compiler is complaining about, whereas it's pretty easy to just import My point is that we should modularise with care, I'm not totally opposed to it and can see some merits. I think if there were |
I think this kinda causes a bit of distraction when observing sources from users' perspective. However it's a minor thing 💁♂️
Can still be partially acheived using
Thats one of the reasons it's draft, I don't like this name either :D
Destructuring probably can be extracted to a separate module as well
I like
|
Yeah, maybe it can be moved under the assumption that the test macros will be separated at some point.
Yeah ok I'm happy with that for now.
Yeah it could be, there is definitely a strong enough distinction. But I'm not sure if there's any value in doing so.
I agree,
Yeah, the way I'm thinking about |
I don't think someone will import destructuring in isolation, so the only value I see is more clear logic separation mostly for development/contributions Alright so for now I'll:
And there is still an issue with docc 🤔 |
Yep sounds good 👌
Hmm yeah that's an important one; I'd be interested to know how |
|
Ah sad, fair enough. Yeah that's fine for now. |
Motivation
Some users may not want to rely on certain APIs, so it's beneficial to divide the package into multiple targets. This allows library users to selectively use features.
Impact
MacroToolkit
target remains to offer a convenient option for importing all the provided targets, so no breaking changes involved and it can be a patch update if mergedNotes
Example is extracted to a separate package to avoid distraction with modules
I would consider using
swift-macro-testing
for tests, its' really convenient, but probably a topic for a separate PR. Here you can find my package build with bothswift-macro-toolkit
andswift-macro-testing
(AssociatedObjectMacro target)Docs are not updated
Reason: Docc does not support multy-target generation atm
Public discussions:
Potential workaround (afaiu for github pages):
color-components/blob/main/.github/workflows/docs
Uses: oss-common-actions/workflows/swift-generate-and-publish-docs
Generates index page: https://sersoft-gmbh.github.io/color-components/latest/