Switch to Idiomatic Swift Naming Convention #114
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @ryansobol, Thank you for your contribution and for introducing those fantastic features: Configuring the Generator and SOAR-0013. These ideas are great! I'm planning to bump the version to 3.0.0 to reflect the breaking changes. MISC: |
wei18
approved these changes
May 4, 2025
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduction
First, I want to thank you for creating and maintaining such an exceptional Swift client for the GitHub REST API. As a Swift developer who has benefited greatly from this library, I'm excited to contribute back to the project. While I'm not a regular contributor, I believe this change could significantly enhance the developer experience by making the generated code feel more natural and idiomatic to Swift developers.
Summary
This PR modifies the OpenAPI generation configuration to use the idiomatic Swift naming strategy instead of the default defensive one. This change transforms generated symbol names to follow Swift's conventional naming patterns rather than preserving special characters with descriptive words (e.g.,
users_sol_list→usersList), significantly enhancing the developer experience.Scope Impact: This change affects virtually all non-trivial Swift symbols throughout the codebase. Due to the comprehensive impact, I recommend this as a breaking change for a v3.0.0 release, making it opt-in for developers who use semantic versioning best practices.
Examples of Name Changes
Function Names
copilot_sol_get_hyphen_copilot_hyphen_organization_hyphen_detailscopilotGetCopilotOrganizationDetailsType Names
copilot_hyphen_organization_hyphen_detailsCopilotOrganizationDetailsEnum Case Names
text_x_hyphen_markdowntextXMarkdownProperty Names
total_engaged_userstotalEngagedUsersParameter Names
team_slugteamSlugRationale & Motivation
Improved Accessibility & Developer Experience
The primary goal of this change is to significantly enhance the usability of the generated Swift client library. The idiomatic naming strategy aligns perfectly with standard Swift practices, making the API feel more natural and intuitive for Swift developers.
Maybe it's just me, but I've found the defensive naming difficult to work with in practice. Methods like
reactions_sol_list_hyphen_for_hyphen_team_hyphen_discussion_hyphen_comment_hyphen_in_hyphen_orgbecome much more readable asreactionsListForTeamDiscussionCommentInOrg. This transformation dramatically improves readability and understanding for developers at all experience levels, making the API more intuitive and reducing cognitive overhead.This change represents a significant accessibility improvement, making the API feel like idiomatic Swift rather than an awkward translation from another language.
Changes Made
Scripts/GeneratorConfigBuilder.swiftto use the idiomatic naming strategypublicsymbol namesServers.Server1.url()APIREADME.mdfile similarlyProposed Migration Strategy
As this change introduces significant, albeit beneficial, breaking changes across the API surface, thoughtful consideration of migration options would be valuable. Semantic versioning principles provide helpful guidance here.
One potential approach could be:
2.xbranch.mainbranch for a future v3.0.0 release as a breaking change.The existing automated build system that monitors GitHub's OpenAPI specification for changes could be leveraged in this scenario - continuing to update and publish v2.x releases with the defensive naming convention from a maintenance branch, while the
mainbranch would generate idiomatic naming for v3.0.0 and beyond. This would provide support for both existing users and those wanting the improved Swift-native experience.That said, maintaining parallel release streams does involve additional overhead, and the maintainers might reasonably prefer to focus solely on the new idiomatic version. There may also be other migration approaches not outlined here that would better fit the project's needs. Ultimately, the decision on versioning strategy rests entirely with the project maintainers.
This pull request focuses solely on implementing the switch to the idiomatic naming convention and regenerating the code accordingly. Any branching strategy or infrastructure changes to support versioning would need to be addressed separately if desired.
Also, supporting both naming conventions simultaneously within the same module would likely introduce significant complexity to the build system and codebase. A one-convention-per-major-version approach provides a cleaner migration path, allowing users to opt-in to the improved naming when ready.
I hope this suggestion contributes constructively to the discussion of how to balance improved developer experience with a smooth transition for existing users.
Swift OpenAPI Generator Upgrade
This PR includes an upgrade from Swift OpenAPI Generator 1.2.1 to 1.7.2. Upgrading to at least version 1.6.0 was a requirement for this PR, as that's when the idiomatic naming strategy was first introduced. The decision to go all the way to 1.7.2 provides additional benefits including improved naming configuration options, Windows support, and Swift 6.1+ compatibility.
Click to see detailed version highlights from 1.2.1 to 1.7.2
Version Highlights
Version 1.3.0 (July 2024):
Version 1.4.0 (October 2024):
Version 1.5.0-1.5.1 (November-December 2024):
Version 1.6.0 (December 2024):
Version 1.7.0 (January 2025):
Version 1.7.1-1.7.2 (March-April 2025):
Verification & Potential Drawbacks
Verification
All tests pass successfully when
swift buildandswift testare run locally. The generated code compiles properly and all functionality remains intact with the new naming convention.NOTE: The following
make installwarnings are present on both themainbranch and this feature branch. They are related to issues in the OpenAPI specification interpretation and are not introduced by this PR.Click to see the make install warnings
NOTE: The following
swift buildwarnings are present on both themainbranch and feature branch. The only difference is the naming conventions (camel case vs snake case). These warnings don't affect functionality and are not introduced by this PR.Click to see the swift build warnings
Potential Drawbacks & Mitigation
There is a risk that future GitHub API elements with names that conflict with Swift keywords or conventions might break the automated generation process. However, the significant improvement in API accessibility and developer experience far outweighs the occasional inconvenience of needing to manually configure name overrides when such edge cases arise.
These edge cases are likely to be rare, and the provided override mechanism gives us a straightforward way to handle them when they appear. For example, necessary overrides for the "+1" and "-1" reactions have been included in this PR, and serve as a model for any future scenarios that may arise.
IMHO, the benefit to developer productivity and code readability makes this trade-off well worth it, especially for a team working with this API on a daily basis.