Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: a typo in _OBSOLETE_MANUAL_BUILD_ORDER everywhere #156

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

MojtabaHs
Copy link
Contributor

I’ve resolved the issue throughout the codebase. However, since it involves a public property, it might be wise to introduce an alias using @available to enable automatic renaming.

Let me know your thoughts.

Also, please remember to run the @swift-ci test.

@jakepetroules
Copy link
Collaborator

However, since it involves a public property, it might be wise to introduce an alias using @available to enable automatic renaming.

This is underscored and undocumented, it's not meant to be a public interface.

Copy link
Collaborator

@jakepetroules jakepetroules left a comment

Choose a reason for hiding this comment

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

@swift-ci test

@mirza-garibovic
Copy link
Contributor

Can you please update the title and description of this PR to specify that it's a typo fix just like in your commit message? And do so for PRs going forward.

Copy link
Collaborator

@neonichu neonichu left a comment

Choose a reason for hiding this comment

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

I agree with @mirza-garibovic on the title, but otherwise this looks good.

@neonichu
Copy link
Collaborator

@swift-ci test

@MojtabaHs MojtabaHs changed the title [SWBCore] Enhance settings macro fix: a typo in the _OBSOLETE_MANUAL_BUILD_ORDER everywhere Feb 13, 2025
@MojtabaHs MojtabaHs changed the title fix: a typo in the _OBSOLETE_MANUAL_BUILD_ORDER everywhere fix: a typo in _OBSOLETE_MANUAL_BUILD_ORDER everywhere Feb 13, 2025
@MojtabaHs
Copy link
Contributor Author

MojtabaHs commented Feb 13, 2025

@mirza-garibovic thank you for the suggestion! Title has been updated as requested.

I will try to reuse the PR title with the commit messages if they reflect similar changes. However, most of the time PRs include multiple changes with several commits (to facilitate easier reviews and any back-and-forth updates) and I need to summarizes the overall changes.

I usually try to cover these in the summary, short and minimally:

  • What Module/Feature changed? e.g. [SWBCore]
  • Why I changed this? e.g to Enhance or refactor or extending code coverage or fix or etc.
  • What part of the Module/Feature changed? e.g. Settings or PIFGenerationModel or etc.
  • What was the overall type of changes? e.g inline documentation, tests, warning, properties, etc.

If there are any guidelines to follow, please let me know. I would be happy to review them.

@mirza-garibovic
Copy link
Contributor

@MojtabaHs thanks! Your thinking is good but summaries should be specific about the nature of the enhancement.

@aciidgh aciidgh merged commit a44c74b into swiftlang:main Feb 14, 2025
1 of 3 checks passed
@MojtabaHs MojtabaHs deleted the SWBCoreSettingsMacro branch February 15, 2025 09:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants