-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
.NET Core 2.2 Runtime #4172
.NET Core 2.2 Runtime #4172
Conversation
Updated to support dotnet:2.2 runtime
Additional documentation, test files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is not needed tests/dat/actions/helloDotNet.bin.
Missing link from https://github.com/apache/incubator-openwhisk/blob/master/docs/actions.md#languages-and-runtimes.
Minor suggestions otherwise.
👍 otherwise.
docs/actions-dotnet.md
Outdated
For example, create a C# project called `Apache.OpenWhisk.Example.Dotnet`: | ||
|
||
```bash | ||
dotnet new classlib -n Apache.OpenWhisk.Example.Dotnet -lang C# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the #
here needs to be escaped or wrapped in quotes otherwise interpreted as a comment in some terminals.
ansible/files/runtimes.json
Outdated
}, | ||
"attached": { | ||
"attachmentName": "codefile", | ||
"attachmentType": "application/zip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be "application/octet-stream"
ansible/files/runtimes.json
Outdated
}, | ||
"attached": { | ||
"attachmentName": "codefile", | ||
"attachmentType": "application/zip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"attachmentType": "application/zip" | |
"attachmentType": "application/octet-stream" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chetanmeh can you review this change?
All of the attachments in the manifest are text/plain which iirc is because we store all blobs as 64 encoded text. So should this also be plain/text
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually is text/plain
Updated attachmentType to match other runtimes
Addresses issue found in item 1 for #4177. dotnet node was one level too low.
@shawnallen85 you need one more change:
i can't push to your branch directly. now the test will pass. |
I'm a diff newbie, but I think this is what @rabbah had suggested.
Ran wskdev fmt to appropriately update the WskUnicodeTests.scala file. wskdev fmt FTW.
Codecov Report
@@ Coverage Diff @@
## master #4172 +/- ##
===========================================
+ Coverage 60.67% 81.17% +20.49%
===========================================
Files 152 152
Lines 7291 7292 +1
Branches 470 472 +2
===========================================
+ Hits 4424 5919 +1495
+ Misses 2867 1373 -1494
Continue to review full report at Codecov.
|
Description
Updated runtimes.json to include the .NET Core 2.2 runtime container.
Additional documentation for creating .NET Core 2.2 actions.
Standard and Unicode tests for .NET Core 2.2.
My changes affect the following components
Types of changes
Checklist: