Skip to content

Conversation

@GrabYourPitchforks
Copy link
Member

This contains the Unicode data files that are needed by dotnet/runtime#328. I followed steps 1 - 3 in the README and verified that this package can be consumed by our test projects.

Open questions:

  1. Do we really want to name this Private.CoreFx.TestUtilities.Unicode.TestData? It will eventually be consumed by several unit projects, including Private.CoreFx.TestUtilities.Unicode, System.Globalization.Tests, and System.Text.Encodings.Web.Tests. (I'll need to make a minor change to this and bump the version to 1.0.1 before that last project can consume it.)

  2. Do we need THIRD-PARTY-NOTICES.TXT? These files are copied from https://www.unicode.org/Public/, and we have a similar notices file in the runtime repo.

@ViktorHofer
Copy link
Member

Yes we need a THIRD-PARTY-NOTICES.TXT. Please see 2a90188 for an example.

Do we really want to name this Private.CoreFx.TestUtilities.Unicode.TestData?

You decide on the name :)

@GrabYourPitchforks
Copy link
Member Author

@ViktorHofer I didn't even see that notices file under the project folder. Should I also throw this one under the new project folder (which I guess means it gets swept up as part of the nupkg?), or do we want to move it to the repo root?

@ViktorHofer
Copy link
Member

I think we should move it to the repo root and include your notice. I think it being part of the package was a mistake.

@GrabYourPitchforks
Copy link
Member Author

Ok, I'll fix it up when I'm back in the office.

@GrabYourPitchforks
Copy link
Member Author

Changes in most recent iteration:

  • Old THIRD-PARTY-NOTICES.TXT merged into new file at repo root.
  • Bumped version of System.IO.Compression.TestData since I technically changed the contents of the package.
  • Renamed CoreFx.Private.TestUtilities.Unicode.TestData to Microsoft.Private.UnicodeData.

@ViktorHofer
Copy link
Member

What about Microsoft.Private.Runtime.UnicodeData to make the intent clear?

@ViktorHofer
Copy link
Member

/azp run runtime-assets

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

The repo is now ready. Feel free to merge after addressing my last comment :) I'm still not 100% sure about the package name. Looping in a few other folks who might be interested: @akoeplinger @jkotas

@GrabYourPitchforks
Copy link
Member Author

System.Private.* works for me. :)

@GrabYourPitchforks GrabYourPitchforks merged commit 4e8d82a into dotnet:master Dec 10, 2019
@GrabYourPitchforks GrabYourPitchforks deleted the unicodedata branch December 10, 2019 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants