Skip to content
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

Blame should offer coloring choices #298

Closed
gitblit opened this issue Aug 12, 2015 · 12 comments
Closed

Blame should offer coloring choices #298

gitblit opened this issue Aug 12, 2015 · 12 comments

Comments

@gitblit
Copy link
Owner

@gitblit gitblit commented Aug 12, 2015

Originally reported on Google Code with ID 2

Blame should have a selector to choose
  * Color by Commit
  * Color by Author
  * Color by Age


Reported by James.Moger on 2011-06-26 16:35:29

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Reported by James.Moger on 2012-12-04 14:26:48

  • Status changed: New

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Hi James,
  I thought I'd have a go at implementing this feature, hopefully I'll be able to implement
what you're looking for. Do you have any specific ideas on how you'd like this implemented,
how the coloured blame should look, etc?

  I've knocked together something just to find my feet for working inside Gitblit and
I've attached a screenshot of my (very) rough attempt. Is it in the ballpark of what
you're looking for or am I way off?

Cheers,
Alex

Reported by alex.lewis001 on 2013-11-22 09:14:34


- _Attachment: ColouredBlameExample.png
![ColouredBlameExample.png](https://storage.googleapis.com/google-code-attachments/gitblit/issue-2/comment-2/ColouredBlameExample.png)_

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Nice.  That looks like what I would have done.  Are you assigning color from a fixed
pool? Or trying to calculate one from the author name?  Either way that looks good.

As for working inside Gitblit... the internal architecture is about to change - what
you've learned is transferable.  I've been refactoring for modularization and dependency
injection.  As you can imagine, the change is massive and touches just about every
class.  It still has all the same "powers" - but access to them is slightly different.
 I hope to land the refactor this weekend.

Reported by James.Moger on 2013-11-22 14:22:25

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Ok I've done a little bit more work in the Color Mapping part. It now generates random
colors, applies a tint to "lighten" them so they don't dominate the page and remembers
that color given a key (a Map). Therefore subsequent invocations will result in the
same color for a given key. In the example above the key is the author thinking that
when I add the ability to select coloring by commit the key will be the commit ID instead.

When it comes to coloring by Age is a random color ok or would you like shades/tints
of a single colour to indicate age (dark = oldest, light = newest)?

Is having a Map of key-to-color ok or would you prefer me to see if I can "calculate"
a color from a key? I noticed my color mapping util needed to be serializable which
concerned my slightly I don't know where it's actually going, how long it's being stored
for etc. which might have memory implications. I was surprised it was serializing in
the first place but that'll just be me not understanding Wicket yet. 

As for your refactoring that shouldn't cause me any problems, I'll be able to reapply
my changes without too much trouble I'm sure.

One more question... how would you prefer I submit my code for review, once I'm done?
GitHub PR, patchsets in an email, etc?

Reported by alex.lewis001 on 2013-11-22 15:08:33

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Any instance or final object in a page must serializable.  This is how Wicket operates
with server-side sessions/pages/etc.  I'm not in love with that so I try to avoid generating
stateful pages but here it might be unavoidable - it might depend on how you want to
trigger the repaint: one option might be to call back to the page with a query parameter
instead of an ajax call.

I think age should be shades of 1 color, otherwise it loses meaning.

A Github PR is my current preference.  When I finish the Tickets feature and hopefully
(someday) have OpenID auth my preference might change to Gitblit-hosted review.  But
that ain't gonna happen in the next few weeks.  :)

Thanks for tackling such an old issue!

Reported by James.Moger on 2013-11-22 15:22:34

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

I'll keep with what I've got for the moment and revisit it later on with the aim of
making it stateless if possible. 

I've started working on the repaint. I've added links which go back to the BlamePage
with a "blame type" parameter. The location of the links is messy at the moment (in
the same row as the commit|line|data headings) but the alignment is wrong. I'd like
to have the links to the right of that header whilst the "commit|line|data" part stays
far left. I'd also probably like to somehow highlight the currently selected blame
type but I'll get to that later :) Do you have any preferences as to where those links
should appear? If so I'll do that instead.

1 color it is. 

Ok hopefully I'll have something ready fairly soon, spare time permitting. Are there
any deadlines to aim for?

No problem! It's a low-risk piece of work which is proving to be a nice introduction
to the Gitblit source, Wicket, etc. and if my code does make it into Gitblit it'll
be really nice to have contributed a feature. 

Reported by alex.lewis001 on 2013-11-22 16:29:58

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

End of December.  No hurry.

Reported by James.Moger on 2013-11-22 18:32:09

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

James, I think I'm in a position to raise a Pull Request for this feature. Shall I go
ahead and raise the PR for you to review it or do you want to take a look at my repo
first?

https://github.com/alexmob/gitblit/tree/issue-2 

Reported by alex.lewis001 on 2013-12-02 15:26:03

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

I took a peek.  Go ahead, create a PR, and we'll do the review dance.  :)

Reported by James.Moger on 2013-12-02 15:33:55

  • Status changed: Started
  • Labels added: Milestone-1.4.0

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Done :)

Reported by alex.lewis001 on 2013-12-02 15:35:27

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

Reported by James.Moger on 2013-12-02 21:09:37

  • Status changed: Queued

@gitblit
Copy link
Owner Author

@gitblit gitblit commented Aug 12, 2015

1.4.0 released.

Reported by James.Moger on 2014-03-09 18:06:21

  • Status changed: Done

@gitblit gitblit closed this as completed Aug 12, 2015
@flaix flaix added this to the 1.4.0 milestone Dec 13, 2016
@flaix flaix added this to the 1.4.0 milestone Dec 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants