-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Git statistics in Activity tab #4724
Conversation
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
a8240f8
to
afa37b6
Compare
Codecov Report
@@ Coverage Diff @@
## master #4724 +/- ##
==========================================
+ Coverage 41.23% 41.24% +0.01%
==========================================
Files 421 422 +1
Lines 58125 58277 +152
==========================================
+ Hits 23965 24037 +72
- Misses 30989 31061 +72
- Partials 3171 3179 +8
Continue to review full report at Codecov.
|
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.
Works ans looks greats.
Just need to move calls to git command to go-gitea/git as it will ease the migration when fully switching to gopkg.in/src-d/go-git.v4. Another solution is to directly replace those calls by gopkg.in/src-d/go-git.v4.
If anyone could point to method to do this with src-d/git I could rewrite this to use it |
@sapk I think we need to move go-gitea/git back as a module of go-gitea/gitea. But we need keep all git codes as a well-formed sub package. |
@lafriks LGTM but would be perfect with tests for new code under models and modules/git |
@sapk test added |
I think your code is right. But when some language's translation is missing, it will be replaced by English. And if on this language it's no difference between singular and plural. Then it will buggy. So we should know the real language before |
@lunny it is only for untranslated strings for Chinese languages. For other it will be correct, and TrN function can not really know if it is untranslated or not. Anyway it is out of scope for this PR |
That is out of this PR but it's still a problem. |
@lunny I agree but by currently uses i18n I don't see how that could be fixed |
@lafriks In french, I need to agree the verb with the grammatical number.
Depending on the verb, I think in english we have to as well.
|
Ok, I will try to split it out a bit more and add additional parts to be translatable depending on count |
Maybe the words "has" and "have" could be removed and it would still have the same meaning without the problem above? ie: Maybe no need for has/have since "pushed" and "changed" already indicate past tense and work for both singular and plural? Not sure if that makes the translation any easier or not... |
@mrsdizzie This will not make it any easier for french. @lafriks |
Fixes #4697
TODO:
Implement bar chart with top 10 users in UI(can be implemented in other PR)Screenshot: