Skip to content

If FoundationEssentials is available use it#158

Merged
ktoso merged 4 commits intoapple:mainfrom
adam-fowler:foundation-essentials
Feb 25, 2026
Merged

If FoundationEssentials is available use it#158
ktoso merged 4 commits intoapple:mainfrom
adam-fowler:foundation-essentials

Conversation

@adam-fowler
Copy link
Copy Markdown
Contributor

Use FoundationEssentials if it is available

Motivation:

Reduce size of any application using swift-metrics on linux

Copy link
Copy Markdown
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

As noted in other repos, this is a semver major change, so we’ll need to work out how to stage this in.

@adam-fowler
Copy link
Copy Markdown
Contributor Author

adam-fowler commented Jan 13, 2025

As noted in other repos, this is a semver major change, so we’ll need to work out how to stage this in.

Is this a breaking change?

It was a breaking change in swift-nio-transport-services because the import NIOFoundationCompat was removed, but removing an import Foundation doesn't make Foundation unavailable as an import in packages dependent on swift-metrics.

@Lukasa
Copy link
Copy Markdown
Contributor

Lukasa commented Jan 13, 2025

Yes, it's a breaking change. Reproducing my comment from swift-crypto:

It doesn’t make the full API available, but it makes some things available. An example of one such problem is available at https://forums.swift.org/t/pitch-fixing-member-import-visibility/71432.

The Swift team consider this a bug, and I agree, but our semver policies take the language as it is, not as it is intended to be. So this will require us to burn a major.

@adam-fowler
Copy link
Copy Markdown
Contributor Author

pfff

@t089
Copy link
Copy Markdown

t089 commented Jan 13, 2025

Yeah :( I wonder if we could get away with it, though. Today, most apps (knowingly or unknowingly) anyhow somehow import Foundation somewhere, so unless you are actively trying to avoid it, you are anyhow already importing it. And if you are actively trying to avoid it, you do not use API from it, so this change will not be source breaking.

@Lukasa
Copy link
Copy Markdown
Contributor

Lukasa commented Jan 13, 2025

And if you are actively trying to avoid, you do not use API from it, so this change will not be source breaking.

There's a certain "if a tree falls in the forest and no-one is around to hear it" quality to this argument. It's true that whether a change is source-breaking is a function of whether anyone is actually using the API. The wrinkle is that it's very hard for us to validate that they aren't. My bias is strongly against taking that risk, usually.

In the case of metrics in particular, there are good reasons to think that we may want to modernise the API. We could do these two things at once, put the more modern API in a new module and deprecate the old interface. This will cause a natural community migration away from the deprecated API and toward a new one, while avoiding a source break.

@ktoso
Copy link
Copy Markdown
Member

ktoso commented Feb 25, 2026

Given the official ESG stance on this, we should move to take this change, and we will not consider this a major bump.

Resolved merge conflicts, kicking off CI, re-requesting reviews.

@ktoso ktoso requested a review from Lukasa February 25, 2026 11:00
@ktoso ktoso added 🆕 semver/minor Adds new public API. and removed ⚠️ semver/major Breaks existing public API. labels Feb 25, 2026
@ktoso
Copy link
Copy Markdown
Member

ktoso commented Feb 25, 2026

@Lukasa I'm going to assume you're ok with us following the ESG conclusion here 🫡

@ktoso ktoso merged commit ba325c3 into apple:main Feb 25, 2026
45 checks passed
@adam-fowler adam-fowler deleted the foundation-essentials branch February 25, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants