Feature #6092
closedTruncate Git revision labels in Activity page/feed and allow configurable length
Added by Phil Moorhouse over 14 years ago. Updated almost 14 years ago.
100%
Description
Git uses hashes for the revision labels rather than an incremental counter, so a lot of space is used up just displaying the label in the activity page / feed that would otherwise show more useful info (such as a longer part of the commit message).
Redmine already truncates the Git revision labels to the first 8 chars on the Repository page, which is more than enough for most projects, so it would be good to see this on the Activity page. If we could also configure the length to be even shorter if required, that would be great. The href for any links would obviously still contain the full revision hash.
Files
Related issues
Updated by Adam Soltys over 14 years ago
- File patch.diff patch.diff added
- Status changed from New to Resolved
- % Done changed from 0 to 100
Attached patch applies the fix. Also available at http://github.com/asoltys/redmine
Updated by Adam Soltys over 14 years ago
Oh, I should mention I didn't implement this as a configurable setting as requested. Maybe later...
Updated by Felix Schäfer over 14 years ago
- Category set to UI
- Status changed from Resolved to New
Please leave the status changes to the devs, thank you. (resolved is for patches in trunk but not in stable yet).
The patch looks good to me, I'm not sure we'd need it configurable either. If we take the git etiquette of 72 chars per line, add the "Revision " and ": " around the revision hash (11 chars total, might be somewhat different in other locales) and 8 chars for the revision hash, we end up with 91 chars, which leaves some space for the other locales to have longer strings.
JB: I like the idea as it is, but I'd rather make a @short_revision
attribute each adapter can override at will rather than a hard default like that. I especially think people using SCMs with sequential numbering would be quite surprised to see commit 10000000
followed by commit 10000000
:-)
Updated by Adam Soltys over 14 years ago
Agreed Felix. I'll work on moving it at least into a variable if not into a setting. Also, sorry about changing the ticket status, I'll know now not to do that anymore.
Updated by Adam Soltys over 14 years ago
- File patch.diff patch.diff added
Ok, I've implemented this as a setting now. I figure this way it's not necessary to have separate values in each adapter. I've defaulted the value to 8. If someone with an svn repository with over 10 million revisions needs to increase this, they can!
Updated by Toshi MARUYAMA over 14 years ago
Adam Soltys wrote:
Ok, I've implemented this as a setting now. I figure this way it's not necessary to have separate values in each adapter. I've defaulted the value to 8. If someone with an svn repository with over 10 million revisions needs to increase this, they can!
Your patch is great!
Can you implement setting of "7.days" at source:tags/1.0.0/app/models/repository/git.rb#L50 and http://www.redmine.org/attachments/3272/git-fast-browse.patch in #4773 ?
Updated by Adam Soltys over 14 years ago
- File patch.diff patch.diff added
Toshi, ok, I did these two things as settings for you, but I don't actually think they should be included in trunk and presented to the average user. They're specific to git, for one thing, and they deal with some pretty obscure internal performance mechanisms.
A better solution in my opinion would be to revisit the git adapter and see if we can get some more performance out of it by default. If github can do it, so can we!
Updated by Adam Soltys over 14 years ago
I just realized I was incorrectly using the log_display_limit setting instead of the truncation setting in a couple places of code. Fixed that. Here's the patch again with just the truncation setting addressing the original issue, this time done properly.
I've also placed this change in a "for-eric" branch on my github fork and sent him a pull request. http://github.com/asoltys/redmine/tree/for-eric
Updated by Adam Soltys over 14 years ago
Updated by Toshi MARUYAMA over 14 years ago
Adam Soltys wrote:
Toshi, ok, I did these two things as settings for you, but I don't actually think they should be included in trunk and presented to the average user. They're specific to git, for one thing, and they deal with some pretty obscure internal performance mechanisms.
7.days problem is reported at #6013 and "git log -n 1 FILE_or_DIR" problem is reported at #5096.
Updated by Adam Soltys over 14 years ago
Thanks for pointing to those issues Toshi. I hadn't been following redmine development for the last year or so but I'm glad to see that other people have taken up the charge on these matters. I was exposed first-hand to some of the general performance problems that redmine has with distributed scm's back when I worked on #1406, so I've gained an appreciation and desire to fix these things once and for all.
I still don't agree with exposing these two settings at the moment because I still see them as temporary hacks to solve performance problems that I think we could do better on. In particular, I have some ideas about possibly pre-fetching and caching some repository history or implementing some more sophisticated on-demand lookups and partial history scans that might give us adequate performance without requiring users to sacrifice the "last revision" field or manually specify how far back to look for dirty changesets.
I need to do a bit more thinking and design-work before I tackle a full-on reimplementation of the git and mercurial adapters though. So in the meantime I'm just trying to get my feet wet again with contributing to redmine. I do have plans to become a regular contributor through the coming fall and winter though and repository management and performance will be a central area of interest for me.
Updated by Jean-Baptiste Barth about 14 years ago
Didn't have the time to read everything but thank you guys for your work on these topics.
About the original issue, I agree we should truncate git's sha1 revision names but I have 2 worries about it :- I don't like the idea of introducing a Setting for that. There are already bunches of useless settings in redmine, and consulting administration panel always give me the feeling that we have too many things here which leads to confusion and bloat software ; I'd rather turn it into a constant in the class in a first approach. I think nobody cares Github truncates revision names to 7 or 8 chars because nobody reads these IDs, they're just here to point to the commit. We should have the same approach, decide a number (6? 7? 8? 10?) and begin with this.
- after some thoughts about it, I think this modification must be applied at adapter level, only to SCM with non sequential revision numbers.
Adam: can you confirm the last patch is the only one to keep ? We can delete other patches so that it's clearer which one to work on.
Updated by Toshi MARUYAMA about 14 years ago
Yuya's implementation of Mercurial overhaul (#4455) shows activity with mercurial style such as 12:ff70262d476d .
You can see at http://dev.openttdcoop.org/projects/ttrs/activity .
Updated by Felix Schäfer about 14 years ago
Jean-Baptiste Barth wrote:
About the original issue, I agree we should truncate git's sha1 revision names but I have 2 worries about it :
- I don't like the idea of introducing a Setting for that. There are already bunches of useless settings in redmine, and consulting administration panel always give me the feeling that we have too many things here which leads to confusion and bloat software ; I'd rather turn it into a constant in the class in a first approach. I think nobody cares Github truncates revision names to 7 or 8 chars because nobody reads these IDs, they're just here to point to the commit. We should have the same approach, decide a number (6? 7? 8? 10?) and begin with this.
- after some thoughts about it, I think this modification must be applied at adapter level, only to SCM with non sequential revision numbers.
Agreed, I'd go with 8 chars and an extra short_revision
virtual attribute in the adapter.
Updated by Toshi MARUYAMA about 14 years ago
Adam Soltys wrote:
In particular, I have some ideas about possibly pre-fetching and caching some repository history or implementing some more sophisticated on-demand lookups and partial history scans that might give us adequate performance without requiring users to sacrifice the "last revision" field or manually specify how far back to look for dirty changesets.
7.days is only git adapter problem.
There are some ideas of git at after note-13 of #4773 .
Because mercurial has revision number, Redmine can know mercurial "last committed revision"(tip) easily.
But mercurial tip is not latest revision (see note-23 of #4455 ).
This is the reason of Redmine 1.0.x mercurial adapter fetching performance problem.
Yuya fixed this problem at note-144 of #4455 and I pushed to my github hg-patches-svntrunk branch .
Updated by Adam Soltys about 14 years ago
Thanks for looking at this Jean-Baptiste. I agree with you about the importance of avoiding software bloat and I don't like exposing obscure technical settings to the users either.
I'll have to look at the code though to see if it's possible to move this setting into the adapters. I think in some places where the revisions are being shortened we don't have access to the adapter objects, which is why I went with a global setting, but maybe I can figure something out. So yes, the latest patch supersedes the other ones, but hold off on looking at it because I'll probably be coming up with another one soon in light of this latest discussion.
Toshi, thanks again for the info. I looked at Yuya's branch quickly and it seems like he's done a lot of good work that I need to take a closer look at.
Updated by Yuya Nishihara about 14 years ago
Adam Soltys wrote:
Hi, in short, my patches try to switch revision formats by repository classes (Mercurial or not).Toshi, thanks again for the info. I looked at Yuya's branch quickly and it seems like he's done a lot of good work that I need to take a closer look at.
Implemented as follows:
- implement Changeset#format_identifier and Revision#format_identifier
- pass changeset or revision object to format_revision and link_to_revision helpers,
so that it can utilize #format_identifier
Updated by Toshi MARUYAMA about 14 years ago
Adam Soltys wrote:
Toshi, ok, I did these two things as settings for you, but I don't actually think they should be included in trunk and presented to the average user. They're specific to git, for one thing, and they deal with some pretty obscure internal performance mechanisms.
A better solution in my opinion would be to revisit the git adapter and see if we can get some more performance out of it by default. If github can do it, so can we!
Github seems to use Ajax for calling "git log -n 1 FILE_or_DIR".
Redmine calls "git log -n 1 FILE_or_DIR" not only in browsing directory tree but also in cat/diff/annotate.
Because entry() of abstract_adapter.rb calls entries() at source:tags/1.0.1/lib/redmine/scm/adapters/abstract_adapter.rb#L84 .
Updated by Toshi MARUYAMA about 14 years ago
- File yuya-issue-4455-mq-ab997af9e-exclude-hg.diff yuya-issue-4455-mq-ab997af9e-exclude-hg.diff added
- File git-act-before.png git-act-before.png added
- File git-act-after.png git-act-after.png added
I imported Yuya's MQ (Mercurial Queues) exclude Mercurial adapter to github and done minor change.
Now my github branch is issue-4455-yuya-mq-20100825-exclude-hg
And I attach the patch and git activity page images of source:tags/1.0.1/test/fixtures/repositories/git_repository.tar.gz.
Updated by Toshi MARUYAMA about 14 years ago
Adam Soltys wrote:
I need to do a bit more thinking and design-work before I tackle a full-on reimplementation of the git and mercurial adapters though. So in the meantime I'm just trying to get my feet wet again with contributing to redmine. I do have plans to become a regular contributor through the coming fall and winter though and repository management and performance will be a central area of interest for me.
Adam, please consider #5357.
Mercurial has same problem (#3567).
Yuya fixed this problem at note-144 of #4455.
Updated by Toshi MARUYAMA about 14 years ago
Updated by Toshi MARUYAMA almost 14 years ago
- File revision-truncate-20101213.diff revision-truncate-20101213.diff added
- File git-12-digits-activity.png git-12-digits-activity.png added
- File git-12-digits-annotate.png git-12-digits-annotate.png added
- File git-12-digits-browse.png git-12-digits-browse.png added
I update the patch to set truncating character number at adapter level.
This patch contains unit tests.
And I pushed to my github.
- https://github.com/marutosi/redmine/commits/git-activity
- https://github.com/marutosi/redmine/commit/710d3f355de44d87ccb3ec0caf49ec77917ca16b
This patch truncates 8 characters.
You can set truncating number.
- blame/annotate
https://github.com/marutosi/redmine/blob/710d3f355de44d87/lib/redmine/scm/adapters/git_adapter.rb#L271 - Others
https://github.com/marutosi/redmine/blob/710d3f355de44d87/app/models/repository/git.rb#L39
I attach truncating 12 characters images.
Updated by Toshi MARUYAMA almost 14 years ago
- File revision-truncate-20101213-fix-fixtures.diff revision-truncate-20101213-fix-fixtures.diff added
revision-truncate-20101213.diff has a lack of fixtures at unit tests.
I fixed it.
revision-truncate-20101213-fix-fixtures.diff is additional patch for revision-truncate-20101213.diff .
And I pushed to my github.
Updated by Toshi MARUYAMA almost 14 years ago
- Category changed from UI to SCM
- Assignee set to Toshi MARUYAMA
Updated by Toshi MARUYAMA almost 14 years ago
- Status changed from New to Closed
- Target version set to 1.1.0
- Resolution set to Fixed