-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Move user related model into models/user #17781
Conversation
568eb75
to
f5fde1f
Compare
Is |
The variables and functions in that files will be shared for other models/user and future models/repo packages. Or we have to create a new sub package to do that. |
OK, then we can have another PR to continue the refactoring, this PR is already so large. LGTM |
f5fde1f
to
91ba9db
Compare
@@ -22,7 +23,6 @@ type LFSLock struct { | |||
ID int64 `xorm:"pk autoincr"` | |||
Repo *Repository `xorm:"-"` | |||
RepoID int64 `xorm:"INDEX NOT NULL"` | |||
Owner *User `xorm:"-"` |
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.
Is the linter able to detect unused members?
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.
The linter is able to detect unused private members, but exported one like these aren't detected.
I think the linter is preventing to detect this, as this Field could be used when go-gitea/gitea is imported as package and a golang program is using that field. But given that go-gitea/gitea shouldn't be used as package(right?), we can modify the deadcode linter and be more aggressive and also check on these Public fields etc. Not sure which refactor PR it was mentioned, but we also could check if a certain function is only used with the xxx_test.go
packages and move it to them.
https://github.com/golangci/go-misc/blob/master/deadcode/deadcode.go
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.
Currently we expect no one imports gitea. Otherwise this change would not be possible as we break the "api".
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.
Got it, I will open up a issue for such proposal. Should I open up a issue at gitea-vet or just here.
make L-G-T-M work |
* Move user related model into models/user * Fix lint for windows * Fix windows lint * Fix windows lint * Move some tests in models * Merge
As title.