Bugzilla – Full Text Bug Listing
| Summary: | [log] Implement APi and UI for Git log --all | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Szymon Brandys <Szymon.Brandys> |
| Component: | Git | Assignee: | Tomasz Zarna <tomasz.zarna> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | janikpiotrek |
| Version: | 0.2 | Flags: | tomasz.zarna:
review+
|
| Target Milestone: | 0.3 M1 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | gsoc2011 | ||
| Bug Depends on: | 343644, 353191 | ||
| Bug Blocks: | 345397, 349727, 353262 | ||
|
Description
Szymon Brandys
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. 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. (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) (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. 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.
(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 |