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(config): add vault provider #191

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

FrankYang0529
Copy link
Contributor

@FrankYang0529 FrankYang0529 commented Nov 2, 2022

ref: spinframework/spin#798, spinframework/spin#661

Content must go through a pre-merge checklist.

Pre-Merge Content Checklist

This documentation has been checked to ensure that:

  • The title, template, and date` are all set
  • File does not use CRLF, but uses plain LF (hint: use cat -ve <filename> | grep '^M' | wc -l and expect 0 as a result)
  • Has passed bart check
  • Has been manually tested by running in Spin/Bartholomew (hint: use PREVIEW_MODE=1 and run npm run styles to update styling)
  • Headings are using Title Case
  • Code blocks have the programming language set to properly highlight syntax and the proper copy directive
  • Have tested with npm run test and resolved all errors

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Thank you for this docs update!

A few grammatical suggestions while this is in draft (pending merge of spinframework/spin#798 as well)...

@flynnduism @mikkelhegn it looks like we're tracking changes to static/data.json -- does this look right to you?

@karthik2804
Copy link
Contributor

@vdice We are tracking static/data.json because we need it to be updated for search indexing.

@mikkelhegn
Copy link
Member

@FrankYang0529 can you run npm run build-index to update the search index and then commit the updated data.json file? We're fixing the PR template to include this step.

@FrankYang0529 FrankYang0529 force-pushed the add-vault-config-provider branch from 2f2fe16 to ac2275f Compare November 3, 2022 13:04
@FrankYang0529
Copy link
Contributor Author

@FrankYang0529 can you run npm run build-index to update the search index and then commit the updated data.json file? We're fixing the PR template to include this step.

Hi @mikkelhegn, I run the command and committed it.

I have a question: how to install bart command? There is a check item in the list. Thank you!

@FrankYang0529 FrankYang0529 marked this pull request as ready for review November 3, 2022 13:08
@mikkelhegn
Copy link
Member

Instructions are here: https://bartholomew.fermyon.dev/quickstart

@FrankYang0529
Copy link
Contributor Author

Instructions are here: https://bartholomew.fermyon.dev/quickstart

Thanks! I also add the reference to pull_request_template.

Copy link
Member

@mikkelhegn mikkelhegn left a comment

Choose a reason for hiding this comment

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

I haven't tested the functionality, but I trust the instructions are ok :-)

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Last thought: We don't yet have a good story around delineating between docs around unreleased Spin changes and docs that are valid for the most recent release (eg v0.6.0 currently). Until then, should we add a note somewhere that the vault config provider functionality is available in the canary release now and anticipated to be included in the next v0.7.0 release?

(Filed #195 around docs versioning strategy)

@FrankYang0529 FrankYang0529 force-pushed the add-vault-config-provider branch from 62c2619 to 5622789 Compare November 4, 2022 14:54
@FrankYang0529
Copy link
Contributor Author

Last thought: We don't yet have a good story around delineating between docs around unreleased Spin changes and docs that are valid for the most recent release (eg v0.6.0 currently). Until then, should we add a note somewhere that the vault config provider functionality is available in the canary release now and anticipated to be included in the next v0.7.0 release?

Added a note for it. Thanks.

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Thank you @FrankYang0529!

@vdice vdice merged commit 8ec8c8f into fermyon:main Nov 4, 2022
@FrankYang0529 FrankYang0529 deleted the add-vault-config-provider branch November 6, 2022 13:36
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