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

Minor improvements (string interpolation in the C# example, removes some blanks) #128

Merged
merged 2 commits into from Feb 9, 2019

Conversation

Projects
None yet
2 participants
@unglaublicherdude
Copy link

unglaublicherdude commented Feb 8, 2019

Description

It does change the C# example to use string interpolation instead of string.Format and removes some blank lines that are indicated by visual studio code.

Which issue(s) this PR fixes

#127
#126

How Has This Been Tested?

I tested it on my local machine.

Types of changes

Minor improvements.

How Has This Been Tested?

I deployed the faas-stack locally with your deploy_stack.sh.

Then I created a new directory. Copied the from the repo templates (I did not use template pull) into it.
Then I did a

$ faas-cli new --lang chsarp helloworld-new
$ faas-cli build -f helloworld-new.yml
$ faas-cli deploy -f helloworld-new.yml
$ echo "foobar" |faas-cli invoke helloworld-new 
Hi there - your input was: foobar

I also did that for the python3 template.

$ faas-cli new --lang python3 helloworld-python
$ faas-cli build -f helloworld-python.yml 
$ faas-cli deploy -f helloworld-python.yml 
$ echo "foobar" | faas-cli invoke helloworld-python
foobar

I have not tested the arm ones but the only functional change is done to the csharp temlates. The other changes are just cosmetic changes.

@derek derek bot added the new-contributor label Feb 8, 2019

@derek

This comment has been minimized.

Copy link

derek bot commented Feb 8, 2019

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off.
That's something we need before your Pull Request can be merged. Please see our contributing guide.

@derek derek bot added the no-dco label Feb 8, 2019

@alexellis alexellis removed the no-dco label Feb 8, 2019

@derek derek bot added the no-dco label Feb 8, 2019

@derek

This comment has been minimized.

Copy link

derek bot commented Feb 8, 2019

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off.
That's something we need before your Pull Request can be merged. Please see our contributing guide.

@unglaublicherdude unglaublicherdude force-pushed the unglaublicherdude:minor-improvements branch 3 times, most recently from c91bd95 to 7c180da Feb 8, 2019

@derek derek bot removed the no-dco label Feb 8, 2019

Matthias added some commits Feb 3, 2019

Changed csharp-templates to string interpolation
String interpolation is more readable and the compiler can decide wether
to use concatination or string.format.

Signed-off-by: Matthias Simonis <github@matthias-simonis.de>
Removes blanks
This commit removes some blanks from a few Dockerfiles and the Readme.

Signed-off-by: Matthias Simonis <github@matthias-simonis.de>

@unglaublicherdude unglaublicherdude force-pushed the unglaublicherdude:minor-improvements branch from 7c180da to bd1b890 Feb 8, 2019

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Feb 8, 2019

Hi, just to check is github@matthias-simonis.de a valid email?

@@ -1,7 +1,7 @@
FROM arm64v8/node:8.9.1

RUN apt-get update -qy \
&& apt-get install -qy curl ca-certificates --no-install-recommends \
&& apt-get install -qy curl ca-certificates --no-install-recommends \

This comment has been minimized.

@alexellis

alexellis Feb 8, 2019

Member

I can't see why these unrelated files have been edited?

This comment has been minimized.

@unglaublicherdude

unglaublicherdude Feb 8, 2019

Author

It is related to #126
If you want, I can make a pull request for each ticket instead of one that does both.

@alexellis
Copy link
Member

alexellis left a comment

LGTM

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Feb 8, 2019

How Has This Been Tested?

Please show some evidence of the testing.

@unglaublicherdude

This comment has been minimized.

Copy link
Author

unglaublicherdude commented Feb 8, 2019

Hi, just to check is github@matthias-simonis.de a valid email?

Hi. Yes, this is a catch-all address I use to track spam send to my addresses.

@unglaublicherdude

This comment has been minimized.

Copy link
Author

unglaublicherdude commented Feb 8, 2019

How Has This Been Tested?

Please show some evidence of the testing.

Yes. I deployed the faas-stack locally with your deploy_stack.sh.

Then I created a new directory. Copied the from the repo templates (I did not use template pull) into it.
Then I did a

$ faas-cli new --lang chsarp helloworld-new
$ faas-cli build -f helloworld-new.yml
$ faas-cli deploy -f helloworld-new.yml
$ echo "foobar" |faas-cli invoke helloworld-new 
Hi there - your input was: foobar

I also did that for the python3 template.

$ faas-cli new --lang python3 helloworld-python
$ faas-cli build -f helloworld-python.yml 
$ faas-cli deploy -f helloworld-python.yml 
$ echo "foobar" | faas-cli invoke helloworld-python
foobar

I have not tested the arm ones but the only functional change is done to the csharp temlates. The other changes are just cosmetic changes.

@unglaublicherdude

This comment has been minimized.

Copy link
Author

unglaublicherdude commented Feb 8, 2019

Ah sorry, I accidentally pushed the close button.

@alexellis
Copy link
Member

alexellis left a comment

LGTM

@alexellis alexellis merged commit 0f2cff5 into openfaas:master Feb 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Feb 9, 2019

Thanks for your PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment