Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.0] For Mac Catalina build, suppress warnings porting to 3.0#39984

Merged
wtgodbe merged 2 commits intodotnet:release/3.0from
buyaa-n:mac_catalina_build_3-0
Aug 7, 2019
Merged

[release/3.0] For Mac Catalina build, suppress warnings porting to 3.0#39984
wtgodbe merged 2 commits intodotnet:release/3.0from
buyaa-n:mac_catalina_build_3-0

Conversation

@buyaa-n
Copy link

@buyaa-n buyaa-n commented Aug 2, 2019

Description

Suppressing warnings for deprecated methods in macOS 10.15 (Porting from master)

Customer Impact

No impact

Regression?

No

Risk

No

@buyaa-n buyaa-n requested review from bartonjs, danmoseley and krwq August 2, 2019 22:03
@krwq
Copy link
Member

krwq commented Aug 2, 2019

@buyaa-n you will also need to fill out the template in the description: https://gist.github.com/danmosemsft/348cbe83ccd7ae0e87c5598223be1380 and get @danmosemsft's approval before merging this

@buyaa-n buyaa-n changed the title For Mac Catalina build, suppress warnings porting to 3.0 [release/3.0] For Mac Catalina build, suppress warnings porting to 3.0 Aug 2, 2019
@danmoseley
Copy link
Member

I'll take it Tuesday for approval

@karelz karelz added this to the 3.0 milestone Aug 3, 2019
if (isServer != 0 && isServer != 1)
return NULL;

#pragma clang diagnostic push
Copy link
Member

Choose a reason for hiding this comment

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

Nit: please add back the whitespace that was there before, e.g. there should ideally be a blank line above this pragma

Copy link
Author

Choose a reason for hiding this comment

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

will update with all similar

Copy link
Author

@buyaa-n buyaa-n Aug 6, 2019

Choose a reason for hiding this comment

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

i assume i should apply it for master first and cherry pick to this PR ...

Copy link
Member

Choose a reason for hiding this comment

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

for such a trivial thing, you need not wait for the same change to merge into master.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, just in case commits need to be identical, but if that is not needed, will update right here

Copy link
Author

Choose a reason for hiding this comment

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

updated here because its easier to review

{
SSLProtocol protocol = PalSslProtocolToSslProtocol(sslProtocol);

#pragma clang diagnostic push
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

{
SSLProtocol protocol = PalSslProtocolToSslProtocol(sslProtocol);

#pragma clang diagnostic push
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
*pOSStatus = SSLCopyPeerTrust(sslContext, pChainOut);
#pragma clang diagnostic pop
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Will stop commenting on these.

@danmoseley
Copy link
Member

Approved by tactics for 3.0, @wtgodbe or @Anipik will merge when branch opens for preview 9.

@wtgodbe
Copy link
Member

wtgodbe commented Aug 7, 2019

Branch is open, merging

@wtgodbe wtgodbe merged commit 6e1a3b2 into dotnet:release/3.0 Aug 7, 2019
@buyaa-n buyaa-n mentioned this pull request Aug 14, 2019
@buyaa-n buyaa-n deleted the mac_catalina_build_3-0 branch November 1, 2019 20:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants