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

Update documentation with service accounts #110

Merged
merged 17 commits into from
Dec 1, 2023

Conversation

volodymyrZotov
Copy link
Collaborator

@volodymyrZotov volodymyrZotov commented Nov 28, 2023

This PR updates documentation to mention Service Accounts support.

It also changes the term '1Password Connect Terraform Provider' to just '1Password Terraform Provider' to cover a wider scope of the tools that the Provider can work with (aka 1Password Connect and 1Password Service Accounts as per now).

Tasks:

  • Update autogenerated docs template templates/index.md.tmpl
  • Update bug report .github/ISSUE_TEMPLATE/bug_report.md

Improve CONTRIBUTE.md:

  • Add Sign Your Commits section.
  • Add Debugging section

Resolves #106

Copy link
Member

@edif2008 edif2008 left a comment

Choose a reason for hiding this comment

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

Code review: ✅

The changes look good and straight forward. Thank you for making sure that the documentation is consistent.

I've left some non-blokcing comments, but once addressed we can merge this. 💪🏻

I've noticed the following in your PR description:

Improve CONTRIBUTE.md:

  • Add Sign Your Commits section.
  • Add Debugging section

Weren't these added as part of #109 already? If so, did we lose that data when merging #99? 🤔

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
@volodymyrZotov
Copy link
Collaborator Author

@edif2008 thank you for the review.
You're right it was done in #109, and that was merged to this branch, not to main. So nothing lost 🙂

We just split the work between me and @williamhpark and prepared this branch with all the documentation changes for the review.

Copy link

@armstrongl armstrongl left a comment

Choose a reason for hiding this comment

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

I added a lot of comments and suggestions here, but please take them all with a grain of salt because I might be technically inaccurate. Additionally, keep in mind that I'll be starting issue #65 soon to add this documentation to the 1Password Developer Portal. After that, we'll need to have a discussion regarding whether we want to keep this information in the repo.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
templates/index.md.tmpl Outdated Show resolved Hide resolved
@volodymyrZotov volodymyrZotov merged commit 439dbd2 into main Dec 1, 2023
4 checks passed
@volodymyrZotov volodymyrZotov deleted the update-documentation-with-service-accounts branch December 1, 2023 19:40
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.

Update documentation to mention that the provider supports Service Accounts.
4 participants