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

funqy docs disclaimers and usage improvements #10693

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

patriot1burke
Copy link
Contributor

There's been some confusion on when to use Funqy, especially with HTTP. Also, the docs were misleading users into thinking that the AwsContext interface could be injected in Lambda environments.

@gastaldi gastaldi added this to the 1.7.0 - master milestone Jul 13, 2020
@gastaldi
Copy link
Contributor

It would be nice to squash these commits before merging

@patriot1burke
Copy link
Contributor Author

Await on #10667 on whether to merge or not.

Copy link
Contributor

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

I think it's a really good addition to the documentation and clearly explain things.
Can you also add the same WARNING in the google-cloud-functions-http` guide ?

I added some questions/comments but nothing very important.

@boring-cyborg boring-cyborg bot added area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/logging area/maven area/vertx labels Jul 14, 2020
Update docs/src/main/asciidoc/funqy.adoc

Co-authored-by: George Gastaldi <[email protected]>

Update docs/src/main/asciidoc/funqy.adoc

Co-authored-by: Loïc Mathieu <[email protected]>

Update docs/src/main/asciidoc/funqy.adoc

Co-authored-by: Loïc Mathieu <[email protected]>

add links
Copy link
Contributor

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

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

LGTM; A good clarification which helps to set expectations right. Thanks, @stuartwdouglas and @patriot1burke. One suggestion inline.


include::./status-include.adoc[]

Follow the link:amazon-lambda-http[Amazon Lambda Http Guide]. It walks through using a variety of HTTP
frameworks on Amazon Lambda, including Funqy.

WARNING: The Funqy HTTP + AWS Lambda binding is not a replacement for REST over HTTP. Because Funqy
Copy link
Contributor

Choose a reason for hiding this comment

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

Small suggestion; I'd put this into an admonition block (here and the other occurrences):

[NOTE] 
====
The Funqy HTTP + AWS Lambda binding...
====

Copy link
Member

Choose a reason for hiding this comment

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

Well, it is an admonition, it just uses the shortcut. I think it's good enough this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/logging area/maven area/vertx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants