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

Add | Adding net8 support #2230

Merged
merged 12 commits into from Jan 12, 2024
Merged

Add | Adding net8 support #2230

merged 12 commits into from Jan 12, 2024

Conversation

JRahnama
Copy link
Member

Replacing Net7 with Net8.0

@JRahnama JRahnama added this to the 5.2.0-preview4 milestone Nov 19, 2023
@codecov Codecov
Copy link

codecov bot commented Nov 19, 2023 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (04b531b) 72.54% compared to head (3f952c3) 72.20%.
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2230      +/-   ##
==========================================
- Coverage   72.54%   72.20%   -0.35%     
==========================================
  Files         310      310              
  Lines       62045    62045              
==========================================
- Hits        45011    44799     -212     
- Misses      17034    17246     +212
Flag Coverage Δ
addons 92.88% <ø> (ø)
netcore 75.98% <ø> (-0.74%) ⬇️
netfx 69.91% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@H-Yeo
Copy link
Contributor

H-Yeo commented Dec 4, 2023

In CertificateTest.cs, do we want to clean up this comment as well?
// Net7.0 the new API added by dotnet runtime will check SANS and then SubjectName
to maybe
// >=Net7.0 the new API added by dotnet runtime will check SANS and then SubjectName

@H-Yeo
Copy link
Contributor

H-Yeo commented Dec 4, 2023

Everything else looks good to me.

@JRahnama
Copy link
Member Author

JRahnama commented Dec 4, 2023

In CertificateTest.cs, do we want to clean up this comment as well? // Net7.0 the new API added by dotnet runtime will check SANS and then SubjectName to maybe // >=Net7.0 the new API added by dotnet runtime will check SANS and then SubjectName

Basically the API was added in net7, so it could stay the same or I can adjust it to net7 and greater version.

@JRahnama
Copy link
Member Author

JRahnama commented Dec 9, 2023

/azurepipelines run

@azure-pipelines Azure Pipelines
Copy link

Pull request contains merge conflicts.

@JRahnama @DavoudEshtehari
Apply suggestions from code review
cd6a31e 
Co-authored-by: DavoudEshtehari <61173489+DavoudEshtehari@users.noreply.github.com>
@JRahnama
Addressing documentation reference to inherit from parent class
f859db0
@JRahnama
Merge branch 'net8' of https://github.com/JRahnama/SqlClient into net8
3f952c3 
# Conflicts:
#	src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.cs
#	src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlException.cs
DavoudEshtehari
DavoudEshtehari reviewed Jan 9, 2024
View reviewed changes
Copy link
Member

@DavoudEshtehari DavoudEshtehari left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for the feedback from the API doc process.

@robertmclaws
Copy link

Hopefully with the new downstream vulnerabilities that have been issues, this package will get updated soon.

Moving forward, this package should contain multiple builds targeting the currently supported runtimes, so that people installing this package in their apps can get the latest release for the .NET runtime they are targeting. I was SHOCKED to see one of my NuGet packages was resolving references all the way back down to .NET 6, even though I was targeting .NET 8, because of this package.

Thanks for your swift attention to getting this resolved!

DavoudEshtehari
DavoudEshtehari approved these changes Jan 12, 2024
View reviewed changes
Copy link
Member

@DavoudEshtehari DavoudEshtehari left a comment

Choose a reason for hiding this comment

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

🎉

@JRahnama JRahnama merged commit eb68e1b into dotnet:main Jan 12, 2024
135 of 148 checks passed
@JRahnama JRahnama mentioned this pull request Jan 21, 2024
Merged
@JRahnama JRahnama deleted the net8 branch May 6, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@campersau campersau campersau left review comments

@David-Engel David-Engel David-Engel approved these changes

@DavoudEshtehari DavoudEshtehari DavoudEshtehari approved these changes

@H-Yeo H-Yeo Awaiting requested review from H-Yeo

Assignees
No one assigned
Labels
netcore
Projects
None yet
Milestone
5.2.0-preview5
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@JRahnama @H-Yeo @robertmclaws @campersau @David-Engel @DavoudEshtehari