Skip to content

Remove Foundation#212

Merged
fpseverino merged 4 commits intovapor:consolekit-5from
fpseverino:remove-foundation
Jun 2, 2025
Merged

Remove Foundation#212
fpseverino merged 4 commits intovapor:consolekit-5from
fpseverino:remove-foundation

Conversation

@fpseverino
Copy link
Copy Markdown
Member

Remove Foundation and use C libraries instead

@fpseverino fpseverino requested review from MahdiBM and ptoffy June 1, 2025 20:13
@fpseverino fpseverino requested review from 0xTim and gwynne as code owners June 1, 2025 20:13
@MahdiBM
Copy link
Copy Markdown
Contributor

MahdiBM commented Jun 2, 2025

I wonder if we really are no longer depending on the full Foundation, considering we don't have these flags enabled to make sure we don't have any "implicit" unintended imports:

        .enableUpcomingFeature("MemberImportVisibility"),
        .enableUpcomingFeature("InternalImportsByDefault"),

perhaps this should be for another PR? The flags might require a decent amount of explicit imports to be added.

@fpseverino
Copy link
Copy Markdown
Member Author

I wonder if we really are no longer depending on the full Foundation, considering we don't have these flags enabled to make sure we don't have any "implicit" unintended imports:

        .enableUpcomingFeature("MemberImportVisibility"),
        .enableUpcomingFeature("InternalImportsByDefault"),

perhaps this should be for another PR? The flags might require a decent amount of explicit imports to be added.

I added the flags, the only change needed was to make the import of Logging (which itself does not use Foundation) and the C libraries public. The C struct tm is used in one of our public structs. Here is the commit

@fpseverino fpseverino requested a review from MahdiBM June 2, 2025 12:23
Copy link
Copy Markdown
Contributor

@MahdiBM MahdiBM left a comment

Choose a reason for hiding this comment

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

LGTM.

@MahdiBM
Copy link
Copy Markdown
Contributor

MahdiBM commented Jun 2, 2025

I'd wait for Tim/Gwynne review as well.

@fpseverino fpseverino merged commit 3473ddb into vapor:consolekit-5 Jun 2, 2025
12 checks passed
@fpseverino fpseverino deleted the remove-foundation branch June 2, 2025 15:14
@fpseverino
Copy link
Copy Markdown
Member Author

I'd wait for Tim/Gwynne review as well.

Whoops. 😅

@MahdiBM
Copy link
Copy Markdown
Contributor

MahdiBM commented Jun 2, 2025

Ah 😅 should be fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants