Skip to content

[NFC] Inject asSwiftSafeName as a dependency #148

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

Merged
merged 3 commits into from
Jul 28, 2023

Conversation

czechboy0
Copy link
Contributor

Motivation

Arguably, we should have done this before, but here we go. In order to allow dynamically customizing the logic for converting OpenAPI to Swift identifiers (in support of staging SOAR-0001 gradually), we need to dependency-inject the (String) -> String function to all the places that need to do this conversion.

Modifications

This PR doesn't change any behavior (NFC - no functional change), it just refactors the code to have a single funnel point through which all the conversion goes, which has access to the Config and can inspect feature flags.

Right now, the default behavior is still the same, but this PR creates room for adding the alternative logic from SOAR-0001.

Result

NFC, prepares room for a subsequent PR.

Now it's possible to conditionalize the Swift name computing logic based on feature flags.

Test Plan

All tests are passing, adapted as needed, created conveniences in tests for getting a default Config, but also made it possible to test the logic by explicitly passing the feature flag.

@czechboy0 czechboy0 requested a review from simonjbeaumont July 27, 2023 12:54
@czechboy0
Copy link
Contributor Author

FYI, @denil-ct, once this lands, you'll be able to add your logic here and then test is by providing the feature flag proposal0001.

Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

I don't think we should be mixing in the injected function with the description types.

Either the names we compute within them should be safed by the thing that constructs them, or the computed properties should be made functions that take a safer, or they should return unsafed names, which the caller then safes.

I think in this way, we might preserve some better layering? WDYT?

@czechboy0 czechboy0 requested a review from simonjbeaumont July 27, 2023 15:42
@czechboy0 czechboy0 merged commit 0fc97c5 into apple:main Jul 28, 2023
@czechboy0 czechboy0 deleted the hd-dependency-inject-asSwiftSafeName branch July 28, 2023 08:24
@czechboy0 czechboy0 added the semver/none No version bump required. label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants