NotificationsYou must be signed in to change notification settings
Fork311
Star927

Merge | Align Type aliases between netfx and netcore#2957

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

benrr101 merged 5 commits intodotnet:mainfromMichelZ:merge-tdsparser-typealiases

Nov 25, 2024

Merged

Merge | Align Type aliases between netfx and netcore#2957

benrr101 merged 5 commits intodotnet:mainfromMichelZ:merge-tdsparser-typealiases

Nov 25, 2024

Conversation

Copy link

Contributor

MichelZ commentedNov 2, 2024

This PR solely aligns type aliases between netfx and netcore versions so they can be merged easier later.

Part of#2953

Align Type aliases between netfx and netcore

47aa343

MichelZ mentioned this pull request

Nov 2, 2024

Work towards merging TdsParser#2953

Open

Align more type aliases

215ed14

benrr101 approved these changes

Nov 4, 2024

View reviewed changes

Copy link

Contributor

benrr101 left a comment

Choose a reason for hiding this comment

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

Looks good, but going forward, I think we can combine a couple of PRs like this together. I'm starting to get fatigue from reviewing these all independently 😅

benrr101 added the Common Project 🚮Things that relate to the common project project label

Nov 4, 2024

Copy link

ContributorAuthor

MichelZ commentedNov 5, 2024

Looks good, but going forward, I think we can combine a couple of PRs like this together. I'm starting to get fatigue from reviewing these all independently 😅

Apologies, I really thought it makes it easier for review to have smaller, more targeted PR's... Guess I was wrong :)
Will combine them in the future

Copy link

Contributor

There's definitely a balance :) I think we'll still take stuff when it's bite-sized like this, but it just might be slow with all the overhead. On the other hand, gigantic PRs are a lot more likely to just be outright rejected

Copy link

Contributor

benrr101 commentedNov 5, 2024

/azp run

Copy link

azure-pipelinesbot commentedNov 5, 2024

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

codecovbot commentedNov 6, 2024•
edited
Loading

Codecov Report

Attention: Patch coverage is86.15385% with9 lines in your changes missing coverage. Please review.

Project coverage is 73.18%. Comparing base(2151501) to head(36fd740).
Report is 33 commits behind head on main.

Files with missing lines	Patch %	Lines
...nt/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs	87.50%	8 Missing⚠️
.../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs	0.00%	1 Missing⚠️

Additional details and impacted files

@@             Coverage Diff             @@##             main    #2957       +/-   ##===========================================- Coverage   92.58%   73.18%   -19.40%===========================================  Files           6      288      +282       Lines         310    65607    +65297     ===========================================+ Hits          287    48014    +47727- Misses         23    17593    +17570