Skip to content

[release/7.0] Mark API as shipped #46042

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
Merged
merged 1 commit into from
Jan 13, 2023
Merged

[release/7.0] Mark API as shipped #46042

merged 1 commit into from
Jan 13, 2023

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Jan 11, 2023

Move the APIs we shipped in 7 from Unshipped.txt to Shipped.txt

@wtgodbe wtgodbe requested a review from a team January 11, 2023 22:16
@wtgodbe wtgodbe added the tell-mode Indicates a PR which is being merged during tell-mode label Jan 11, 2023
dougbu
dougbu reviewed Jan 12, 2023
View reviewed changes
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Stopped after a dozen or so files because this is difficult to view w/o the corresponding Unshipped files.

src/Caching/SqlServer/src/PublicAPI.Shipped.txt Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't the Unshipped.txt files all being emptied in this PR❔

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, I just ran ./eng/scripts/mark-shipped.cmd and committed everything (except the submodule change). Can dig in more today.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think in this case it just moved lines around?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, just moving lines around because the script sorts things alphabetically.

@dougbu
Copy link
Contributor

dougbu commented Jan 12, 2023

Just curious: What was your process here❔ For example, did you run eng\scripts\mark-shipped.cmd then git add PublicApi*.txt❔

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 12, 2023

Just curious: What was your process here❔ For example, did you run eng\scripts\mark-shipped.cmd then git add PublicApi*.txt❔

Yes

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 12, 2023

Huge number of errors here, we may need to update the mark-shipped script. Or, if this is mostly because of the weird nullability behavior w/ the analyzer (which doesn't seem to be getting much support from the roslyn team), we might want to consider switching to some combination of ApiCompat/PackageValidation SDK like dotnet/runtime does. @ViktorHofer any thoughts on what it would take to do that?

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 12, 2023

Scratch that, I think the errors are all because the script did a weird thing with spacing (when moving lines to *.shipped.txt, it put them all on the same line). Will see if I can figure out why, rework the script, then re-run it.

wtgodbe
wtgodbe commented Jan 12, 2023
View reviewed changes
@wtgodbe wtgodbe mentioned this pull request Jan 12, 2023
Merged
dougbu
dougbu approved these changes Jan 12, 2023
View reviewed changes
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Looked through ~20% of the files and didn't see anything to worry about.

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 13, 2023

Only failures are in Code Check about modifying baseline fails - merging

@wtgodbe wtgodbe merged commit b6bd323 into dotnet:release/7.0 Jan 13, 2023
@wtgodbe wtgodbe added this to the 7.0.3 milestone Jan 13, 2023
@wtgodbe wtgodbe deleted the wtgodbe/Api7 branch January 13, 2023 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy

@halter73 halter73 Awaiting requested review from halter73

@Tratcher Tratcher Awaiting requested review from Tratcher

@JamesNK JamesNK Awaiting requested review from JamesNK

@captainsafia captainsafia Awaiting requested review from captainsafia

@javiercn javiercn Awaiting requested review from javiercn

1 more reviewer

@dougbu dougbu dougbu approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Milestone
7.0.3
Development

Successfully merging this pull request may close these issues.
2 participants
@wtgodbe @dougbu