Skip to content

Handle SetModel error, fixes one errcheck report (#257) #589

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 12, 2017
Merged

Handle SetModel error, fixes one errcheck report (#257) #589

merged 1 commit into from
Feb 12, 2017

Conversation

strk
Copy link
Member

@strk strk commented Jan 5, 2017

I guess it could be also backported to 1.0

@lunny
Copy link
Member

lunny commented Jan 5, 2017

I think the admin command is not very necessary? we can do it on the UI.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 5, 2017
@strk
Copy link
Member Author

strk commented Jan 5, 2017

you mean you would drop the runCreateUser function ?

@Bwko
Copy link
Member

Bwko commented Jan 11, 2017

I agree with @lunny , maybe we should drop the admin command.

@lunny
Copy link
Member

lunny commented Feb 7, 2017

But for this one. I will give it LGTM.

@tboerger tboerger 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 Feb 7, 2017
@lunny lunny added this to the 1.1.0 milestone Feb 7, 2017
@lunny lunny added the type/enhancement An improvement of existing functionality label Feb 7, 2017
@minecrafter
Copy link
Contributor

I do see uses for the admin command outside of just batch account creation:

  • reset a user's password from the command line
  • disable two-factor authentication from an account in the event that the device is stolen and/or missing, they forget their scratch code, and the end user can verify they are the owner

@lunny
Copy link
Member

lunny commented Feb 9, 2017

@minecrafter Yes. Command admin is sometimes useful.

@bkcsoft
Copy link
Member

bkcsoft commented Feb 12, 2017

LGTM

@tboerger tboerger 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 Feb 12, 2017
@bkcsoft bkcsoft merged commit 9b4f6c1 into go-gitea:master Feb 12, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Feb 12, 2017

@strk bug-fix so yes, please backport :)

@strk
Copy link
Member Author

strk commented Feb 12, 2017

I'm away from my main development machine, could anyone cherry-pick this to 1.0 and verify ?

@lunny lunny mentioned this pull request Feb 16, 2017
Merged
@tboerger tboerger added the backport/done All backports for this PR have been created label Feb 21, 2017
@strk strk deleted the errcheck-admin branch March 11, 2017 09:52
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@strk @lunny @Bwko @minecrafter @bkcsoft @tboerger