Skip to content

Set copyright and package license#264

Merged
tmat merged 1 commit intomasterfrom
dev/tomat/copyrights
Feb 22, 2019
Merged

Set copyright and package license#264
tmat merged 1 commit intomasterfrom
dev/tomat/copyrights

Conversation

@tmat
Copy link
Member

@tmat tmat commented Feb 21, 2019

Prepares repo for change dotnet/arcade#2003 by setting Copyright and PackageLicenseExpression properties. These values will be required to be set by each repository once dotnet/arcade#2003 is merged.

In order to not break the current builds this change sets the properties conditionally. This condition can be removed once all repos switch to Arcade that has dotnet/arcade#2003.

@markwilkie

@krwq krwq requested a review from safern February 21, 2019 04:43
@safern safern requested a review from joperezr February 21, 2019 23:26
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM, but I know @joperezr imported the props at the end for a specific reason, plus set the MicrosoftCompiler property to false for something specific that I don't remember.

@tmat
Copy link
Member Author

tmat commented Feb 21, 2019

UsingToolMicrosoftNetCompilers is false by default. It used to be different, so now it's not necessary to override it anymore.

The recommendation is to import Sdk.props first:

The Sdk.props file sets various properties and item groups to default values. It is recommended to perform any customizations after importing the SDK.

BTW, setting <Language>C#</Language> seems unnecessary. The language is determined from the project extension (e.g. .csproj).

@tmat
Copy link
Member Author

tmat commented Feb 21, 2019

OT: All the logic in /build.proj seems unnecessary as well. This is replicating/circumventing similar Arcade build logic. Why not have a solution that contains all the projects and build that solution?

@tmat tmat merged commit fcbeb8f into master Feb 22, 2019
@tmat tmat deleted the dev/tomat/copyrights branch February 22, 2019 17:18
@safern
Copy link
Member

safern commented Feb 22, 2019

BTW, setting <Language>C#</Language> seems unnecessary. The language is determined from the project extension (e.g. .csproj).

Agreed.

OT: All the logic in /build.proj seems unnecessary as well. This is replicating/circumventing similar Arcade build logic. Why not have a solution that contains all the projects and build that solution?

I think @joperezr wanted control over what targets are run on which projects and he's concerned that this repo is potentially going to grow to a handful amount of projects as they add bindings and drivers, potentially something like corefx, over 500 projects, from last time I spoke to him, that where the insights he told me.

@tmat
Copy link
Member Author

tmat commented Feb 22, 2019

@safern @joperezr I'd rather avoid replicating CoreFX build structure unless absolutely necessary and instead I'd strongly suggest to follow what all other repos are doing. We have 180 projects in Roslyn repo and there haven't been any issues. If any issues arise we need to address them in the product and/or in Arcade SDK, not in each repo separately.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2023
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.

2 participants