Skip to content

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Jan 12, 2023

Took all of the .shipped files from #46042, built the repo, and fixed errors one-by-one.

@dougbu
Copy link
Contributor

dougbu commented Jan 12, 2023

Took all of the .shipped files from #46042, built the repo, and fixed errors one-by-one.

I checked git diff wtgodbe:wtgodbe/Api7..wtgodbe:wtgodbe/Api8 -- **/PublicApi.Shipped.txt and got no output. Am I correct you haven't (yet) had to update the Shipped files for nullability reasons❔

But, from the CI errors, it looks like some symbols are missing from the Unshipped files still…

I also get the same lack of output from ... -- **/PublicApi.Unshipped.txt. Since the wtgodbe/Api7 Unshipped files are empty (I checked), are the right files in wtgodbe/Api8❔

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 12, 2023

I checked git diff wtgodbe:wtgodbe/Api7..wtgodbe:wtgodbe/Api8 -- **/PublicApi.Shipped.txt and got no output. Am I correct you haven't (yet) had to update the Shipped files for nullability reasons❔

Correct, I didn't encounter any instances where I'd have to do this. Which makes sense, if those nullability changes should never have been in Unshipped in the first place, this PR wouldn't have to touch them (if they're already in Shipped, they'll stay there)

But, from the CI errors, it looks like some symbols are missing from the Unshipped files still…

Argh, yep, something's wrong. Weird, it passed for me locally. Will take a look.

I also get the same lack of output from ... -- **/PublicApi.Unshipped.txt. Since the wtgodbe/Api7 Unshipped files are empty (I checked), are the right files in wtgodbe/Api8❔

Something's up there, there's definitely differences - e.g.:

https://github.com/wtgodbe/aspnetcore/blob/wtgodbe/Api7/src/Components/Components/src/PublicAPI.Unshipped.txt
https://github.com/wtgodbe/aspnetcore/blob/wtgodbe/Api8/src/Components/Components/src/PublicAPI.Unshipped.txt

@wtgodbe wtgodbe enabled auto-merge (squash) January 13, 2023 18:11
@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 13, 2023

Seems to working for me now locally

Copy link
Contributor

@mkArtakMSFT mkArtakMSFT left a comment

Choose a reason for hiding this comment

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

I've spot-checked some of the changes. Thanks @wtgodbe !

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.

Checked a few things locally:

  1. Very few (7) Shipped files differ from what's now in release/7.0. The differences relate to
  2. Renaming for the net7.0 -> net8.0 transition
  3. Slight ordering changes in renamed files
  4. Newly-nullable public surface
  5. The new src/Servers/Kestrel/Transport.NamedPipes/src/ project
  6. 24 Unshipped files contain various changes. Most seem to be removals or small additions. That's fine as long as the CI agrees.

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 13, 2023

Only failure is the expected one in Code Check - merging

@wtgodbe wtgodbe disabled auto-merge January 13, 2023 20:21
@wtgodbe wtgodbe merged commit a403939 into dotnet:main Jan 13, 2023
@wtgodbe wtgodbe deleted the wtgodbe/Api8 branch January 13, 2023 20:21
@ghost ghost added this to the 8.0-preview1 milestone Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants