Skip to content

Conversation

@MaesterChestnut
Copy link
Contributor

No description provided.

@MichaelDoyle
Copy link
Member

MichaelDoyle commented May 10, 2024

I suggest we split this into 2 PRs.

One that ensures that the "deployment" related docs work as intended with genkit init (these are firebase.md, cloud-run.md and deploy-node.md), and another that does more of the snippet standardization work.

@MichaelDoyle MichaelDoyle changed the title Modify several examples of inconsistent theme Update deploy and auth documentation to use menuSuggestionFlow May 13, 2024
Copy link
Member

@MichaelDoyle MichaelDoyle left a comment

Choose a reason for hiding this comment

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

Made a few comments.

Please pay particular attention to variable names that need to change along with the flow. I highlighted ones that I spotted, but there are likely more.

Copy link
Member

Choose a reason for hiding this comment

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

Should this still take "banana" as input?

Copy link
Member

Choose a reason for hiding this comment

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

Should this still take "banana" as input?

docs/auth.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Should this still take "banana" as input?

Copy link
Member

Choose a reason for hiding this comment

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

(if we're still going w/ menuSuggestionFlow)

docs/auth.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little torn on using menuSuggestionFlow here, since the rest of the doc before and after this point refer to selfSummaryFlow. I'm wondering if we should use the same (i.e. selfSummaryFlow) throughout.

Copy link
Member

Choose a reason for hiding this comment

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

There are still some references below that need to be updated.

L129-131 - subject matter doesn't work as well w/ menuSuggestionFlow

L162-166 curls jokeFlow w/ banana as input.

docs/auth.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Reference to (joke) a few more lines below as an input param under authPolicy.

Copy link
Member

@MichaelDoyle MichaelDoyle left a comment

Choose a reason for hiding this comment

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

image

@MaesterChestnut MaesterChestnut merged commit 2d1427f into main May 13, 2024
@MaesterChestnut MaesterChestnut deleted the cc/docSampleUpdates branch May 13, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants