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

Add config possibility to set pass path #171

Merged
merged 5 commits into from
May 26, 2018
Merged

Add config possibility to set pass path #171

merged 5 commits into from
May 26, 2018

Conversation

dvogt23
Copy link
Contributor

@dvogt23 dvogt23 commented Apr 27, 2018

hey,

have some small changes in config possibilities in the usage with pass.

greet

@coryb
Copy link
Contributor

coryb commented May 24, 2018

Hey @dvogt23, sorry I missed this PR. Could you explain the use-case for this? I was wondering if it made more sense to completely override the password path, rather than just the GoJira prefix? Maybe something like this:

if o.PasswordSource.Value == "pass" {
	if o.PasswordPath.Value != "" {
		return o.PasswordPath.Value
	}
	return fmt.Sprintf("GoJira/%s", user)
}

I am wondiner if the PasswordPath variable should be renamed, perhaps something like PasswordName? I am concerned that PasswordPath would be confusing with PasswordDirectory since "path" and "directory" are often synonyms.

@dvogt23
Copy link
Contributor Author

dvogt23 commented May 24, 2018

Hey @coryb thanks for feedback and this good suggestions.

In my use case, i have already an entry for the jira login and in my opinion, fixed paths/names can be better optionally moved to a config.

So who should make those changes? I could change my commit, or?

@coryb coryb merged commit fa01ff5 into go-jira:master May 26, 2018
@coryb
Copy link
Contributor

coryb commented May 26, 2018

I have updated the code and merged it in.
Using password-name property, and it will be used to completely override the password entry. So the default value would be something like:

password-name: GoJira/username@example.com

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.

2 participants