Skip to content

Conversation

@jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Jul 7, 2020

Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139604

Keeping Third-Party IP in a source control system separate from
Microsoft source code and other assets is the preferred engineering
practice…

Begin following this preferred engineering practice: move source code
which was copied into this repo into a separate src-ThirdPartyIP
directory, so that it is easier to track and manage them.

Move the NUnitLite sources into src-ThirdPartyIP\NUnitLite.

Move the force-net/crc32.net sources into
src-ThirdPartyIP\crc32.net.

Move the bazelbuild/bazel sources into
src-ThirdPartyIP\bazel.

Move the C# ported android/platform/tools/base sources into
src-ThirdPartyIP\android-platform-tools-base.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Is there another MS repo that uses this src-ThirdPartyIP directory as a convention?

Would src/ThirdPartyIP also work? Then we wouldn't clutter the top-level directory.

@jonpryor jonpryor force-pushed the jonp-separate-3rd-party-contents branch from d7738bf to 587abf0 Compare July 7, 2020 21:04
@jonpryor
Copy link
Contributor Author

jonpryor commented Jul 7, 2020

@jonathanpeppers asked:

Is there another MS repo that uses this src-ThirdPartyIP directory as a convention?

Nope. The bug suggests:

  1. Create a directory and name it “OpenSource”, “Proprietary”, “ThirdPartyIP”, or similar naming convention depending on the type of Third-Party IP you are using

"OpenSource" and "Proprietary" didn't seem quite useful to me…

Would src/ThirdPartyIP also work? Then we wouldn't clutter the top-level directory.

While true, it also doesn't make it immediately obvious from the top-level where the 3rd party code resides. My reading of the bug is that it should be obvious; it should be top-level.

I'm open to alternate naming suggestions. I guess I could go with ThirdPartyIP instead of src-ThirdParytIP

@grendello
Copy link
Contributor

grendello commented Jul 7, 2020

@jonathanpeppers asked:

Is there another MS repo that uses this src-ThirdPartyIP directory as a convention?

Nope. The bug suggests:

  1. Create a directory and name it “OpenSource”, “Proprietary”, “ThirdPartyIP”, or similar naming convention depending on the type of Third-Party IP you are using

"OpenSource" and "Proprietary" didn't seem quite useful to me…

Would src/ThirdPartyIP also work? Then we wouldn't clutter the top-level directory.

While true, it also doesn't make it immediately obvious from the top-level where the 3rd party code resides. My reading of the bug is that it should be obvious; it should be top-level.

I'm open to alternate naming suggestions. I guess I could go with ThirdPartyIP instead of src-ThirdParytIP

My first throught whenever I see the acronym IP is "Internet Protocol", not anything related to any form of source code. I think we need something that suggests it's a source directory, src-ThirdParty (from 1. above it seems IP is not required) would work for me - it is sorted next to src, suggests that there are sources inside and identifies them as 3rd party.

@jonpryor jonpryor force-pushed the jonp-separate-3rd-party-contents branch from 587abf0 to c3a328d Compare July 8, 2020 01:46
@jonpryor
Copy link
Contributor Author

jonpryor commented Jul 8, 2020

Renamed to src-ThirdParty.

@dellis1972
Copy link
Contributor

Should this PR also move aapt2 and manifestmergertooling, since they are technically third party.

@jonpryor
Copy link
Contributor Author

jonpryor commented Jul 8, 2020

Should this PR also move aapt2 and manifestmerger tooling, since they are technically third party.

The concern is around source code, not binary artifacts. Otherwise we'd have to have all @(PackageReference)s "elsewhere", which would be ridiculous.

Thus, do we have source for aapt2 and manifestmerger in this repo? If so, yes.

However, my understanding is that those tools come from "elsewhere", e.g. manifestmerger comes via Gradle. Instructions to "import" manifestmerger are in this repo, but that's not the same thing. :-)

Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139604

> Keeping Third-Party IP in a source control system separate from
> Microsoft source code and other assets is the preferred engineering
> practice…

Begin following this preferred engineering practice: move source code
which was copied into this repo into a separate `src-ThirdParty`
directory, so that it is easier to track and manage them.

Move the NUnitLite sources into `src-ThirdParty\NUnitLite`.

Move the force-net/crc32.net sources into
`src-ThirdParty\crc32.net`.

Move the bazelbuild/bazel sources into
`src-ThirdParty\bazel`.

Move the *C# ported* android/platform/tools/base sources into
`src-ThirdParty\android-platform-tools-base`.
@jonpryor jonpryor force-pushed the jonp-separate-3rd-party-contents branch from c3a328d to b1aac82 Compare July 8, 2020 14:08
@jonpryor jonpryor merged commit a452af2 into dotnet:master Jul 8, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants