Skip to content

Add username flag in create-user command #6534

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

Merged
merged 6 commits into from
Apr 9, 2019

Conversation

ngourdon
Copy link
Contributor

@ngourdon ngourdon commented Apr 7, 2019

Ensure consistency of name/username flags between create-user and change-password commands

  • Add username flag in create-user command
  • Deprecate name flag in create-user command

fix #6174

@codecov-io
Copy link

codecov-io commented Apr 7, 2019

Codecov Report

Merging #6534 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6534      +/-   ##
==========================================
- Coverage   40.35%   40.34%   -0.01%     
==========================================
  Files         405      405              
  Lines       54251    54251              
==========================================
- Hits        21895    21890       -5     
- Misses      29340    29345       +5     
  Partials     3016     3016
Impacted Files Coverage Δ
modules/log/event.go 64.46% <0%> (-1.53%) ⬇️
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e949db...8606fdd. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 7, 2019
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Apr 7, 2019
@lafriks lafriks added this to the 1.9.0 milestone Apr 7, 2019
cmd/admin.go Outdated
return errors.New("Cannot set both --name and --username flags")
}
if !c.IsSet("name") && !c.IsSet("username") {
return errors.New("One of --name and --username flags must be set")
Copy link
Member

Choose a reason for hiding this comment

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

Not

Suggested change
return errors.New("One of --name and --username flags must be set")
return errors.New("One of --name or --username flags must be set")

@mrsdizzie
Copy link
Member

docs/content/doc/usage/command-line.en-us.md should be updated as part of this PR as well to document the change (probably fine to just change name to username there)

@mrsdizzie
Copy link
Member

Hmm actually, since this is tagged 1.9.0 the docs should probably include mention of both options since they aren't versioned at all (I forget!). People downloading a current release wouldn't have access to this change. Maybe something like "As of gitea 1.9.0, use the --username flag instead"

@ngourdon
Copy link
Contributor Author

ngourdon commented Apr 7, 2019

If I understand well, the doc on gitea.io comes from the master branch.
I will improve the doc as you say to be fine for both versions.

Something like
--name value: Username. Required. As of gitea 1.9.0, use the --username flag instead
--username value: Username. Required. New in gitea 1.9.0

And for the example, keeping the --name flag and adding a comment
gitea admin create-user --name myname --password asecurepassword --email me@example.com
As of gitea 1.9.0, use the --username flag instead

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 9, 2019
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 9, 2019
@ngourdon
Copy link
Contributor Author

ngourdon commented Apr 9, 2019

Do you want me to squash commits to clean history before merging ?

@techknowlogick
Copy link
Member

@ngourdon oh no. Don’t worry about that, we will do that. Thanks for PR :)

@ngourdon
Copy link
Contributor Author

ngourdon commented Apr 9, 2019

My pleasure 😄
Thanks to you all.

@techknowlogick techknowlogick merged commit 2b9b331 into go-gitea:master Apr 9, 2019
@lunny lunny added the type/deprecation Previously provided functionality is removed label Apr 9, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/deprecation Previously provided functionality is removed type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--name and --username consistency issue in CLI
8 participants