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.

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

feat!: rename faas to function #210

Merged
merged 2 commits into from Nov 6, 2020
Merged

Conversation

zroubalik
Copy link
Contributor

@zroubalik zroubalik commented Nov 5, 2020

Signed-off-by: Zbynek Roubalik zroubali@redhat.com

This PR renames:

  • binary: faas -> function
  • env variables: FAAS_ -> FUNCTION_
  • config: faas.yaml -> func.yaml
  • release registry: quay.io/boson/faas -> quay.io/boson/function

Fixes: #201

@zroubalik zroubalik requested review from rhuss and a team November 5, 2020 14:58
@lance
Copy link
Member

lance commented Nov 5, 2020

release registry: quay.io/boson/faas -> quay.io/boson/faas

I assume this is a typo 😄

@zroubalik
Copy link
Contributor Author

zroubalik commented Nov 5, 2020

release registry: quay.io/boson/faas -> quay.io/boson/faas

I assume this is a typo 😄

Yeah :) I've updated the description

Copy link
Member

@lance lance left a comment

Choose a reason for hiding this comment

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

Looks good with a few small changes. The big thing is that I should land #202 and you should rebase, then do pkger. The Node.js and Quarkus changes you have made here won't get picked up without it. :)

README.md Outdated Show resolved Hide resolved
cmd/root.go Outdated Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
templates/go/http/README.md Outdated Show resolved Hide resolved
templates/node/events/package.json Outdated Show resolved Hide resolved
templates/node/events/test/integration.js Outdated Show resolved Hide resolved
@zroubalik
Copy link
Contributor Author

@lance thanks for the review, fixed

@lance
Copy link
Member

lance commented Nov 5, 2020

@zroubalik lgtm - unless @rhuss has any concerns let's do it!

Copy link
Contributor

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me (but see my comments).

Alson not sure about that pkged.go file (can't add a comment there). What is this ?

Beside this (and reading the changes), I wonder whether we should add at least func as an alias as this is much shorter and would be a perfect drop-in replacement for faas when it comes to the length of the command. Also I would be in favor of a func.yaml over function.yaml.

docs/getting_started_kubernetes.md Outdated Show resolved Hide resolved
cmd/root.go Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
config.go Outdated Show resolved Hide resolved
@@ -1,8 +1,2837 @@
{
"name": "event-handler",
"version": "0.1.0",
"lockfileVersion": 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file supposed to be part of this renaming PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm, don't know how does this end up here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these files get modified after running make test. The deps needed to be updated.

Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
@zroubalik
Copy link
Contributor Author

@rhuss Thanks for the review. I have fixed the things from your comments, and renamed faas.yaml -> func.yaml.
func alias would be nice, but we can cover it in the separate PR imho, if we agree on it.

Is it ok to merge?

docs/commands.md Outdated Show resolved Hide resolved
docs/commands.md Outdated Show resolved Hide resolved
Copy link
Contributor

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

looks good but there are still some references to function.yaml

docs/commands.md Outdated Show resolved Hide resolved
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
@rhuss
Copy link
Contributor

rhuss commented Nov 6, 2020

Thanks !

/lgtm
/approve

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

Successfully merging this pull request may close these issues.

Rename binary to function
3 participants