-
Notifications
You must be signed in to change notification settings - Fork 502
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
chore: update top-level password manager docs #4159
chore: update top-level password manager docs #4159
Conversation
22312f9
to
aa71420
Compare
aa71420
to
ea1df3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution.
Most of this information is already in Templating and doesn't need repetition.
It would be better to provide a link to templating than to repeat this information here.
Co-authored-by: Austin Ziegler <[email protected]>
Co-authored-by: Austin Ziegler <[email protected]>
Co-authored-by: Austin Ziegler <[email protected]>
Co-authored-by: Austin Ziegler <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve left one more suggestion. I’d like @twpayne’s input on this as well, but I think that this works reasonably well.
You’ll need to squash this down to a single commit that follows the Chezmoi conventional commit rules.
@@ -1,6 +1,30 @@ | |||
# Password manager integration | |||
# Password Manager Integration in Chezmoi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the in Chezmoi
; it doesn't match any of the other titles in the user guide.
Sounds good @halostatue, thanks for the feedback. The reason I put that much info in the password manager index.md initially is because I hadn't know about the existence of (nor understood) the templating capabilities at all when wanting to use Chezmoi with 1Password. It wasn't clear that I needed to know that to use the password manager integration, either. So rather than assuming the user has a full understanding of templates and how they work with Chezmoi, I thought it would be helpful to have a bit more of an overview for folks like me who ignored/skipped a bit of the docs that they didn't think were relevant to them. When wanting to integrate Chezmoi with 1Password I didn't want to "figure out templating", I just wanted to be able to dynamically reference a password from 1Password in my source file. It was not clear initially that I needed to fully understand how templating worked before being able to use the password manager integration. Hopefully my latest changes provide a bit more guidance for folks that are coming to the docs with the password manager integration as their entry point. |
I can't put as much detail in this comment (on my phone), but I disagree that there should be basic detail like using Something like Giving an example with that link provides context and the next place to look for more details on templating, but repeating basic details covered in depth elsewhere isn't ideal. |
@halostatue obviously you're free to do whatever you want. I'm just giving you the feedback as a user who had been using Chezmoi for its basic functionality (totally avoiding/oblivious to templates) for several months, when I wanted to start using the 1password integration I came to the docs specifically about password integrations and was lost. The verbiage is important, and while it was obvious that I could add curly braces to get Chezmoi to read data from 1Password using "template functions", it wasn't at all clear that this was part of a broader templating capability of the tool and required converting existing files to template files for it to work. Reading the docs about how to use it with password managers had me update the source with the template functions, but I assumed that the presence of the "template functions" was enough for Chezmoi to see them and use them. That is not the case without changing the extension of the file. I strongly feel that you should meet users where there are provide the relevant information for them based on their entry point to the docs. Having a bit of extra info about how templating applies in the password manager situation is important to make sure folks realize that they need to understand templates more generally and that the password manager capabilities are an implementation of that. |
That, however, is the entire point of the Templating document. Needlessly repeating information from that document has several downsides:
The changes you have at e16b1ec are strictly better than the changes at cbdc6fb because they're more concise and do not repeat information that is expressed better elsewhere. Those changes are, however, missing the link to the templating documentation (which should be relative,
Meeting users where they are does not necessarily mean repeating basic information everywhere, but having sufficient cross-linking that getting to the right place is relatively obvious. All of this password-managers/index.md:7–19 repeats information that is better described in the templating documentation (and if it's not, then that documentation should be improved) and assumes that someone entered the chezmoi documentation through the same place you did. If someone entered through Machines / General, should we repeat the same details about how templates work? Of course not. We should probably change the I agree that there is room for improvement in the documentation, but what you have right now is worse than what you had a few hours ago, and I think is strictly worse than what is present now because it has details which are not relevant to mose users of chezmoi. |
Thank you for your contribution here @jondkinney . I agree with @halostatue that the documentation is better improved by adding links to the existing documentation on how to use templates, rather than repeating it, so I'll close this PR. Feel free to open a new PR with links to the existing documentation. |
@twpayne why close this one though? We can still iterate here, no? |
It was very difficult for me to wade through the documentation to find out how to use a password manager with Chezmoi. The key piece that I wasn't able to easily figure out is that the source file needed to be a template for the
chezmoi apply
command to interpolate the template string{{ cmd }}
.Hopefully this top-level documentation will help others that are in a similar position.