Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Publish experimental VersionInfo API #5001

Merged
merged 3 commits into from
Jan 8, 2025
Merged

Publish experimental VersionInfo API #5001

merged 3 commits into from
Jan 8, 2025

Conversation

Scottj1s
Copy link
Member

@Scottj1s Scottj1s commented Jan 3, 2025

Reverts "Make VersionInfo [Experimental] until it is fully working (#3542)"

@codendone
Copy link
Contributor

Has this API been through API review, or is this just prep for when API review completes?

@Scottj1s
Copy link
Member Author

Scottj1s commented Jan 3, 2025

Has this API been through API review, or is this just prep for when API review completes?

I assumed it had been reviewed before, and was made experimental because it was broken (due to missing clcompile preprocessor definitions), per #3542. I'll confirm.

@DrusTheAxe
Copy link
Member

DrusTheAxe commented Jan 3, 2025 •
edited
Loading

Has this API been through API review, or is this just prep for when API review completes?

I assumed it had been reviewed before, and was made experimental because it was broken (due to missing clcompile preprocessor definitions), per #3542. I'll confirm.

Yes it went thru review. Spec's history mentions it
0616493
...

  • Incorporated API Review feedback
    ...

And...there we go. The PR with API Review comments
https://github.com/microsoft/WindowsAppSDK/pull/2893/files/60f13b9818a47d98fe6fa538a2d4b6cf37cc0d03

You're cleared for flight captain. Safe travels :-)

DrusTheAxe
DrusTheAxe approved these changes Jan 3, 2025
View reviewed changes
Copy link
Member

@DrusTheAxe DrusTheAxe left a comment

Choose a reason for hiding this comment

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

LGTM

@DrusTheAxe DrusTheAxe added this to the 1.7 milestone Jan 3, 2025
@DrusTheAxe
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Scottj1s
Copy link
Member Author

Scottj1s commented Jan 7, 2025

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

codendone
codendone requested changes Jan 8, 2025
View reviewed changes
test/VersionInfo/VersionInfoTests.cpp Outdated Show resolved Hide resolved
@Scottj1s
Copy link
Member Author

Scottj1s commented Jan 8, 2025

/azp run

@Scottj1s Scottj1s requested a review from codendone January 8, 2025 01:20
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Scottj1s Scottj1s enabled auto-merge (squash) January 8, 2025 01:59
@Scottj1s Scottj1s merged commit 80e530d into main Jan 8, 2025
26 checks passed
@Scottj1s Scottj1s deleted the user/sjones/publish_versioninfo_api branch January 8, 2025 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codendone codendone codendone approved these changes

@DrusTheAxe DrusTheAxe DrusTheAxe approved these changes

Assignees
No one assigned
Labels
area-VersionInfo
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
3 participants
@Scottj1s @codendone @DrusTheAxe