Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: update Dockerfile to .NET SDK 3.1, improve NuGet metadata #880

Merged
merged 6 commits into from
Oct 16, 2019
Merged

feat: update Dockerfile to .NET SDK 3.1, improve NuGet metadata #880

merged 6 commits into from
Oct 16, 2019

Conversation

RomainMuller
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@RomainMuller RomainMuller requested a review from a team as a code owner October 14, 2019 19:47
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@costleya costleya self-assigned this Oct 14, 2019
costleya
costleya previously requested changes Oct 14, 2019
View reviewed changes
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@RomainMuller RomainMuller force-pushed the rmuller/upgrade-dotnet-sdk branch from 9d3db88 to ffbb767 Compare October 15, 2019 07:26
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@RomainMuller RomainMuller force-pushed the rmuller/upgrade-dotnet-sdk branch from ffbb767 to a1f544d Compare October 15, 2019 10:35
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@RomainMuller RomainMuller changed the title feat: Update Dockerfile to .NET SDK 3.1, improve NuGet metadata feat: update Dockerfile to .NET SDK 3.1, improve NuGet metadata Oct 15, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@RomainMuller RomainMuller requested a review from costleya October 15, 2019 13:02
costleya
costleya reviewed Oct 15, 2019
View reviewed changes
Copy link
Contributor

@costleya costleya left a comment

Choose a reason for hiding this comment

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

Did we miss substituting the icon at https://github.com/aws/jsii/pull/880/files#diff-00c352eb6411da9e1c9ce8db3759a3a4R84?

Just making sure we are consistent. Besides that, ready to ship at present.

docs/configuration.md
@@ -246,7 +246,7 @@ the standard [`nuget push`][nuget-push] command.

[NuGet]: https://www.nuget.org
[nuget-push]: https://docs.microsoft.com/fr-fr/nuget/nuget-org/publish-a-package
[.NET documentation]: https://docs.microsoft.com/en-us/dotnet/core/tools/csproj#packagelicenseurl
[.NET documentation]: https://docs.microsoft.com/en-us/dotnet/core/tools/csproj#packageiconurl
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem right. I think the #link was extraneous to begin with. Remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah the link is in fact used a couple of lines above to point people at where Microsoft tells you what icons they suggest you use. It's not absolutely necessary, but it's actually kinda useful (as at least this can be expected to remain current on what Microsoft recommends)

@RomainMuller
Copy link
Contributor Author

Regarding the logo update - I reckon you're talking about the jsii-calc packages? In this case, those are currently intentionally left this way - there's some tricky things to do so we can support embedded icons for generated modules... And that deserves it's own separate work item.

@RomainMuller RomainMuller dismissed costleya’s stale review October 16, 2019 07:58

Informal comment notes the concerns are now addressed & this can ship.

@RomainMuller RomainMuller merged commit 5e076cf into master Oct 16, 2019
@RomainMuller RomainMuller deleted the rmuller/upgrade-dotnet-sdk branch October 16, 2019 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@costleya costleya costleya left review comments

Assignees

@costleya costleya

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RomainMuller @aws-cdk-automation @costleya