Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 353088 - [log] Implement APi and UI for Git log --all
Summary: [log] Implement APi and UI for Git log --all
Status: RESOLVED FIXED
Alias: None
Product: Orion
Classification: ECD
Component: Git (show other bugs)
Version: 0.2   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 0.3 M1   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard: gsoc2011
Keywords:
Depends on: 343644 353191
Blocks: 345397 349727 353262
  Show dependency tree
 
Reported: 2011-07-26 05:48 EDT by Szymon Brandys CLA
Modified: 2012-01-19 11:46 EST (History)
1 user (show)

See Also:
tomasz.zarna: review+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Szymon Brandys CLA 2011-07-26 05:48:28 EDT
I would implement it first before we start looking at the graph (bug 345397). There could be an action per repo to show this log on the repo view. The log view could offer a combo to select which branch should be highlighted.
Comment 1 Piotr Janik CLA 2011-07-27 10:23:03 EDT
Server: https://github.com/pjanik/orion.server/tree/bug353088
UI in progress.

I wrote all this code and have the rights to contribute it to Eclipse under the
eclipse.org web site terms of use.
Comment 2 Piotr Janik CLA 2011-07-27 10:42:36 EDT
Small update in server since my last post.

https://github.com/pjanik/orion.server/tree/bug353088
https://github.com/pjanik/orion.client/tree/bug353088

I wrote all this code and have the rights to contribute it to Eclipse under the
eclipse.org web site terms of use.
Comment 3 Tomasz Zarna CLA 2011-07-27 11:18:13 EDT
(In reply to comment #2)
> https://github.com/pjanik/orion.server/tree/bug353088

I don't think we need an extra link[1] added to all files[2]. I was hoping that we could get away with adding a single link (CommitLocation) to a clone[3]. I'm not sure where in the UI we want it, but I thought the link was supposed to be available on the Repositories page.

[1] org.eclipse.orion.server.git.GitConstants.KEY_LOG_ALL
[2] org.eclipse.orion.server.git.GitFileDecorator.addGitLinks(URI, JSONObject)
[3] org.eclipse.orion.server.git.servlets.GitCloneHandlerV1.toJSON(Entry<IPath, File>, URI)
Comment 4 Piotr Janik CLA 2011-07-27 15:36:42 EDT
(In reply to comment #3)
> I don't think we need an extra link[1] added to all files[2]. I was hoping that
> we could get away with adding a single link (CommitLocation) to a clone[3]. I'm
> not sure where in the UI we want it, but I thought the link was supposed to be
> available on the Repositories page.
> 
> [1] org.eclipse.orion.server.git.GitConstants.KEY_LOG_ALL
> [2] org.eclipse.orion.server.git.GitFileDecorator.addGitLinks(URI, JSONObject)
> [3] org.eclipse.orion.server.git.servlets.GitCloneHandlerV1.toJSON(Entry<IPath,
> File>, URI)

Yes, I agree. Link in GitFileDecorator is unnecessary, so I removed it. Updated version is available at github.
Comment 5 Tomasz Zarna CLA 2011-07-28 06:06:15 EDT
Thanks for the update. Both branches look fine, but here are my comments I would like to share with you:
* server: I still don't think we need GitConstants.KEY_LOG_ALL. Can't we just use good, old GitConstants.KEY_COMMIT?
* server: The workaround for missing "git log --all" in JGit is neat, but have you filed a bug against JGit about it at the same time? We're trying to mark all workarounds like this with a reference to a bug in JGit, so in the future, when they fix it, we could replace with a proper code.
* server: New converters in BaseToCloneConverter are fine, but wouldn't it be simpler to add them by doing something like this: "public static final BaseToCloneConverter COMMIT = STATUS;" ?
* server: If you asked me I would add a comment somewhere around "if (path.segment(0).equals("file")) {" GitCommitHandlerV1.handleGet(...) saying it's a special case for "git log all", so we don't get confused with all those ifs.
* client: Do we actually need the getCloneFileUri function (git-log.js)? Can we take the URI from the clone object? Just asking. I've noticed that we already crafting some URIs on the page, but it doesn't mean we cannot do better in this case.
Comment 6 Piotr Janik CLA 2011-07-28 08:31:08 EDT
(In reply to comment #5)
> Thanks for the update. Both branches look fine, but here are my comments I
> would like to share with you:
> * server: I still don't think we need GitConstants.KEY_LOG_ALL. Can't we just
> use good, old GitConstants.KEY_COMMIT?
> * server: The workaround for missing "git log --all" in JGit is neat, but have
> you filed a bug against JGit about it at the same time? We're trying to mark
> all workarounds like this with a reference to a bug in JGit, so in the future,
> when they fix it, we could replace with a proper code.
> * server: New converters in BaseToCloneConverter are fine, but wouldn't it be
> simpler to add them by doing something like this: "public static final
> BaseToCloneConverter COMMIT = STATUS;" ?
> * server: If you asked me I would add a comment somewhere around "if
> (path.segment(0).equals("file")) {" GitCommitHandlerV1.handleGet(...) saying
> it's a special case for "git log all", so we don't get confused with all those
> ifs.
Fixed.
> * client: Do we actually need the getCloneFileUri function (git-log.js)? Can we
> take the URI from the clone object? Just asking. I've noticed that we already
> crafting some URIs on the page, but it doesn't mean we cannot do better in this
> case.
Yes, I need it. 

Updated version:
https://github.com/pjanik/orion.server/tree/bug353088
https://github.com/pjanik/orion.client/tree/bug353088