Skip to content

addressing some missing feedback Relating to Browser#38968

Merged
Anipik merged 4 commits into
dotnet:masterfrom
Anipik:followUp
Jul 10, 2020
Merged

addressing some missing feedback Relating to Browser#38968
Anipik merged 4 commits into
dotnet:masterfrom
Anipik:followUp

Conversation

@Anipik

@Anipik Anipik commented Jul 9, 2020

Copy link
Copy Markdown
Contributor

Related PR #37944

@ghost

ghost commented Jul 9, 2020

Copy link
Copy Markdown

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

Comment thread src/libraries/System.Net.Http/src/System.Net.Http.csproj Outdated
Comment thread src/libraries/System.Data.Odbc/ref/System.Data.Odbc.ODBC32.cs Outdated
Comment thread src/libraries/System.Net.Http/src/System.Net.Http.csproj Outdated
Comment thread src/libraries/System.Net.Http/src/System.Net.Http.csproj Outdated

@jkotas jkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@jkotas

jkotas commented Jul 9, 2020

Copy link
Copy Markdown
Member

Could you please resolve conflicts?

Comment thread src/libraries/System.Net.Http/src/System.Net.Http.csproj Outdated
Comment thread src/libraries/System.Data.Odbc/src/System/Data/Odbc/ODBC32.Common.cs Outdated
Anipik and others added 3 commits July 9, 2020 10:44
@Anipik

Anipik commented Jul 9, 2020

Copy link
Copy Markdown
Contributor Author

@safern @ericstj @akoeplinger can you take another look ? i made some changes. (they are small but no harm in getting an another pair of eyes)

</ItemGroup>
<!-- Unix -->
<ItemGroup Condition=" '$(TargetsUnix)' == 'true' and '$(TargetsAndroid)' != 'true' and '$(TargetsiOS)' != 'true' and '$(TargetstvOS)' != 'true' and '$(TargetsBrowser)' != 'true'">
<ItemGroup Condition="'$(TargetsUnix)' == 'true' and '$(TargetsAndroid)' != 'true' and '$(TargetsiOS)' != 'true' and '$(TargetstvOS)' != 'true'">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: you can simplify this with TargetsMobile

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i will make this change in the next follow up pr for browser

@Anipik Anipik merged commit b62b16a into dotnet:master Jul 10, 2020
@Anipik Anipik deleted the followUp branch July 10, 2020 06:45

@ViktorHofer ViktorHofer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@akoeplinger akoeplinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
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.

9 participants