Skip to content
This repository was archived by the owner on Apr 12, 2020. It is now read-only.

Implemented a tree browser #7

Merged
merged 8 commits into from
Feb 27, 2013
Merged

Implemented a tree browser #7

merged 8 commits into from
Feb 27, 2013

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Feb 12, 2013

Not finished yet

I decided to not use code mirror since the repository is
in a read only mode. I will add highligh.js in a second step.

Todo:

[]: Move some code to the theme

@lyrixx
Copy link
Member Author

lyrixx commented Feb 12, 2013

Need this PR to work fine: gitonomy/gitlib#32

@lyrixx
Copy link
Member Author

lyrixx commented Feb 13, 2013

@alexandresalome can you do a code review ?

I does not like the code in the controller and in the template to render tree.
I have ported every thing from gitonomy.

Why I did not like:

  • The controller is a bit fat
  • The template need a lot of data (i think the commit is useless, see below)
  • The code is very hard to read and totally no intuitive
  • I Think there are design issues in gitlib:
    • Move (or duplicate) Commit::getLastModification to Tree::getLastModification (so is should be in the repository with proxy method)

    • :

      $tree = $repository->getRevision($revision)->getResolved()->getTree();
      $element = $tree->resolvePath($path);
      

      is very weird. I don't understand what happen here. Something like $repository->getTree($rev, $path) could be awesome.

What do you think ?

@alexandresalome
Copy link
Member

I'll have a look at it by the end of the week

@alexandresalome
Copy link
Member

Since I already implemented it for gitonomy, I understand the point of passing lot of information to the template, but every bit of it is required.

I consider it as good-enough to be merged, it's a good start point.

@lyrixx
Copy link
Member Author

lyrixx commented Feb 18, 2013

Ok.
But about

$tree = $repository->getRevision($revision)->getResolved()->getTree();
$element = $tree->resolvePath($path); 

Can we do something ?

I did not try gitonomy, but we should move the git profiler to Gitlib\Bridge\Symfony to make it work with silex.

I think the tree browse use a lot of git call.

alexandresalome pushed a commit that referenced this pull request Feb 27, 2013
Implemented a tree browser
@alexandresalome alexandresalome merged commit 77818f5 into master Feb 27, 2013
@alexandresalome alexandresalome deleted the feat-tree branch February 27, 2013 19:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants