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

Make BitOperations public (CoreFX)#35606

Merged
jkotas merged 12 commits intodotnet:masterfrom
grant-d:grant-d.bitops-ref
Mar 2, 2019
Merged

Make BitOperations public (CoreFX)#35606
jkotas merged 12 commits intodotnet:masterfrom
grant-d:grant-d.bitops-ref

Conversation

@grant-d
Copy link
Copy Markdown

@grant-d grant-d commented Feb 26, 2019 •
edited
Loading

See related: dotnet/coreclr#22864 (comment)

This is the first time I'm working the CoreFX ref assemblies.
Please let me know if/what I have missed.

cc @jkotas

@grant-d grant-d changed the title Make BitOperations public (CoreFX) WIP: Make BitOperations public (CoreFX) Feb 26, 2019
@grant-d grant-d changed the title WIP: Make BitOperations public (CoreFX) [WIP] Make BitOperations public (CoreFX) Feb 26, 2019
danmoseley
danmoseley reviewed Feb 27, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

@ahsonkhan shouldCLSCompliantAttribute appear in ref assys (for new API in this case)

@ahsonkhan
Copy link
Copy Markdown

ahsonkhan commented Feb 27, 2019

shouldCLSCompliantAttribute appear in ref assys (for new API in this case)

Yes.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 27, 2019

Could you please also move and fixup the tests ? I am disabling them in #35615

@grant-d
Copy link
Copy Markdown
Author

grant-d commented Feb 27, 2019

Could you please also move and fixup the tests

Sure, will do

@grant-d grant-d changed the title [WIP] Make BitOperations public (CoreFX) Make BitOperations public (CoreFX) Feb 27, 2019
@grant-d
Copy link
Copy Markdown
Author

grant-d commented Feb 27, 2019 •
edited
Loading

Build is failing due to:
D:\a\1\s\.packages\microsoft.dotnet.genfacades\1.0.0-beta.19126.3\build\Microsoft.DotNet.GenFacades.targets(93,5): error : Did not find type 'T:System.Numerics.BitOperations' in any of the seed assemblies. [D:\a\1\s\src\System.Runtime.Extensions\src\System.Runtime.Extensions.csproj]

Am I missing a file or assembly reference?

@tannergooding
Copy link
Copy Markdown
Member

tannergooding commented Feb 27, 2019

@grant-d, in addition to the changes mirroring back (#33615), we need a PR picking up a new build of CoreCLR (a newer PR of #35586, that actually gets merged).

@grant-d
Copy link
Copy Markdown
Author

grant-d commented Feb 27, 2019

Ah, thanks Tanner.

@stephentoub
Copy link
Copy Markdown
Member

stephentoub commented Mar 1, 2019

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Mar 1, 2019

Azure Pipelines successfully started running 4 pipeline(s).

jkotas
jkotas reviewed Mar 1, 2019
View reviewed changes
Comment thread src/System.Runtime.Extensions/tests/System.Runtime.Extensions.Tests.csproj Outdated
@jkotas
Copy link
Copy Markdown
Member

jkotas commented Mar 1, 2019

Also, could you please add TypesMustExist : Type 'System.Numerics.BitOperations' does not exist in the implementation but it does exist in the contract. to src\System.Runtime.Extensions\src\ApiCompatBaseline.netcoreappaot.txt and src\System.Runtime.Extensions\src\ApiCompatBaseline.uapaot.txt to fix the UWP_NETNative_x86_Release leg

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Mar 1, 2019

Could you please also change GenFacadesIgnoreMissingTypes line in src\System.Runtime.Extensions\src\ System.Runtime.Extensions.csproj to:

<GenFacadesIgnoreMissingTypes Condition="'$(TargetsAOT)'=='true' OR '$(TargetGroup)' == 'uap'">true</GenFacadesIgnoreMissingTypes>

This should fix the "Packaging All Configuration".

The Linux failure looks like an intermittent bug. Filled https://github.com/dotnet/corefx/issues/35708

jkotas
jkotas approved these changes Mar 2, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@jkotas jkotas merged commit 0ae011a into dotnet:master Mar 2, 2019
@grant-d grant-d deleted the grant-d.bitops-ref branch March 2, 2019 05:06
@grant-d
Copy link
Copy Markdown
Author

grant-d commented Mar 2, 2019

Thanks again for your help Jan

@grant-d grant-d mentioned this pull request Mar 5, 2019
Closed
@karelz karelz added this to the 3.0 milestone Mar 18, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@grant-d @jkotas
Make BitOperations public (CoreFX) (dotnet/corefx#35606)
500c684 
* Add ref assembly class

* Add [CLSCompliantAttribute(false)]

* Change name to System.Numerics.BitOperations

* Move ref to System.Runtime.Extensions

Commit migrated from dotnet/corefx@0ae011a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tannergooding tannergooding Awaiting requested review from tannergooding

2 more reviewers

@danmoseley danmoseley danmoseley left review comments

@jkotas jkotas jkotas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

7 participants

@grant-d @ahsonkhan @jkotas @tannergooding @stephentoub @danmoseley @karelz