crew-cr / crew

Code review tool for git projects

Crew is a code review tool for git projects.
It works with a simple API.
So, you can easily integrate it in your workflow.
See the site or the wiki for more information.

crew's Issues

SQlite support

I imagine it's not so difficult, unless you have exotic SQL commands.

the link 'Markdown help' opens the help in the same tab

Which method to install vendors ?

I want to install Atoum in Crew for unit testing

I can use further methods to install Atoum or sfAtoumPlugin

• extract vendor into code => no maintenance
• use git submodule
• use composer

Which one is the best for you ?

Site interne de Crew

Je détiens actuellement le nom de domaine http://crew-cr.org ainsi que le contenu du site public sur un de mes repos privés. Or, le nom de domaine expire demain.
Et là, questions :
Quid de Crew ? Qui l'utilise réellement (pas moi, on a réussi à passer sur GitHub Enterprise) ? Le projet est-il donc mort ? As-t-il un avenir ? (cette question se pose aussi pour la revue de code chez PMSIpilot )
Aucun des contributeurs de la branch master ne l'utilise encore (ou bref...).

Ne devrait-on pas créer une organisation Crew ?

Qu'en pensez-vous ?

ping @ratibus @oziks @lotholf @srogier @pmartin @agallou

Splitting files by directory

Instead of a cut filepath in a feature to be reviewed, I think it would be more appropriate to have directories separation :

../../foo/bar.php
../../foo/foo.php
..../foo/bar/bar.php
..../foo/bar/foo.php

Would be printed as

[/full/path/to/foo]
bar.php
foo.php
[/full/path/to/bar]
bar.php
foo.php

This would be really useful on huge features to quickly see what kind of files are to be checked.

For example with a propel based project we would quickly see wich files are generated OM classes.

I find it hard with the actual setup to quickly notice files cause the directory is trimmed to the left, and file path are right aligned. So the eye cannot see quickly if consecutive files belongs to the same directory.

wiki - installation - apache mod headers missing

Hi,

I'm juste trying to use crew on my workstation. I didnt have apache mod_headers installed and the vhost configuration given at https://github.com/pmsipilot/Crew/wiki/How-to-install-Crew is failing on line "Header append Vary User-Agent env=!dont-vary"

Perhaps you should add another Ifmodule condition inside the first one or precise apache required module, something like this works for me

BrowserMatch ^Mozilla/4 no-gzip
BrowserMatch \bMSIE\s7 !no-gzip
BrowserMatch \bMSIE.*SV !no-gzip
BrowserMatch \bOpera !no-gzip
SetOutputFilter DEFLATE

Header append Vary User-Agent env=!dont-vary

Thanks

"View file" action does not render HTML files properly

When I use "View file" feature, if my file contains HTML, the page is messed up.
Some parts of the file is interpreted as HTML directly.

Comment : '>' in code => double-html-escape

In a comment, I write this :

aaa `$blah->plop()` bbb

i.e. : I input a portion of code, with a > in it ; and I put all that between backticks, to highlight the fact that it's source-code -- and to have it presented as such.

The result I get when the comment is displayed, look like this :

aaa $blah->plop() bbb

Basically : the > I typed is displayed as >
(The $blah->plop() is displayed as code, which is OK -- I just didn't know how to show that here)

There seems to be a bit too much of html-encoding in the code segments.

"Status" on a comment

It would be nice if one could, especially when working a file for which more that reviewee will fix things :

• Mark a comment as "assigned" to himself (so others don't start fixing something that's already being fixed)
• Mark a comment as "fixed"
• Mark a comment as "won't fix"

Delete a review via API

Like the "reviewRequest" API method, it could be great if you also provide a simple "reviewDelete" method that remove the review from the list, without deleting the branch.

What do you think of it ?

Markdown > underscores => too much italic

We often use underscores in our variables names -- like ma_variable_underscore.

If I write that variable name in a crew comment, the underscores are not visible, and "variable" is in italic -- basically, the underscores are interpreted.
IMHO, the underscores should not be interpreted when inside a word (or maybe a more complex rule), to make the markdown parser more "code compliant".

As a matter of facts, according to http://blog.stackoverflow.com/2009/10/markdown-one-year-later/ it is what's done on stackoverflow and github

memory limit during init

>./symfony crew:init
>> file+     config/generated-sfGuardPlugin-schema.xml
>> file-     /home/ABC-OBJECTIF/adrien.gallou/Projets/Crew/plugins/sfGuardPlugin/config/generated-sfGuardPlugin-schema.xml
>> propel    Running "om" phing task
PHP Fatal error:  Allowed memory size of 33554432 bytes exhausted (tried to allocate 97 bytes) inplugins/sfPropelORMPlugin/lib/vendor/propel/generator/lib/model/Table.php on line 701

the task should increase the memory limit (the user should'nt user have to increase the memory in his config).

Files are not ordered during the review

Neither the blue flle list nor the "Next file" / "Previous file" seem to order files.

It's a bit confusing when you come from the branch view where files are ordered.

port to sf2

Projects list: highlight projects with pending reviews

When you have a couple dozen projects in crew, and one of those has a pending review (new status on a branch), it's quite hard to find out on which project the pending review is.

Actually, at work, we have a dashboard that says "1 pending review", but it doesn't indicate on which project/branch that is; so we have to open crew, go to the projects list, and click on each project until we find the one that has a branch marked as "new" -- which is not that fun at all.

Would it be possible to have some kind of an indicator on the projects list, to highlight the projects that have pending reviews?
Either by putting projects with pending reviews on top of the list; or by adding some visual indicator such as an icon?

always display filename instead of last commit message

When reviewing a file, the last commit message is always displayed in the header.

It would be more relevant to display filename instead of last commit message.

Improve the layout (with less.js)

• install and use less.js
• add a title on each icon
• change "thumb up", "thumb down" and "trash" icons
• check readability on Ratibus's screen :-)
• manage new links on diff screen :
  • link to the whole file
  • links to move to next or previous file

Auto focus on field when adding a comment

Possible to create empty comments

It is possible to create empty comments (at a file level).

Considering those are not really that helpful, maybe they should be forbidden? (as they are at a line-level, actually)

It is possible to remove comments from other users

I went through the url to delete a comment of a colleague.
And then, magically or not, the comment was deleted !

Next file should open next non binary file

if the diff contains binary files, the "next file" button opens a blank page if the next file is a binary file.

Next file should open next non binary file.

Empty Repository?

Hi, I just stumbled upon this awesome project and quickly got it installed. Everything seems to be working properly, except that when I add my repository, it doesn't seem to be detecting it's contents. It just shows up as empty.

The only installation issue I see is that the sample apache config refers to the following path:

/path/to/your/workspace/Crew/cache/packed

Except the 'packed' directory does not exist for me.

I checked the logs dir but did not see anything created.

Are there any troubleshooting steps I can follow?

Thanks

View binary files

Currently, it is not possible to review (not even view) binary files.

I admit that viewing binary files is not the main goal of crew; but when an image is added to a webpage (and, so, is added to VCS), it could be useful to view it, at least to check that it's OK (the is no secret on a screenshot, for example).

Campfire notifications don't work when there's a quote in the message

Open file/line in an IDE

(I am not the one doing the review, but the one who is trying to fix what's been written as comments during the review)

When I am on the "diff" screen, and have a comment on a file+line, I generally want to open that file in my IDE, on the line where the comment is, to fix it.

It would be really useful if each line in crew had a link formatted so clicking on it would trigger the opening of the right file, at the right line, in the IDE.

In crew, you'd have to: - Generate a link with a specific protocol, like `phpide://` - Include the file path and line

For example, a link could look like this : phpide:///path/to/my/project/web/my-file.php@65

Then, it's only a matter of (on the developper's computer, not in crew):

• Configuring the browser so the phpide:// protocol is associated with a specific script
• Creating that script so it launches the IDE, passing it the file name and line

Couple of points:

• In crew, you don't know the full path to the file on the dev's computer ; just use a local one, and let the script on the developper's computer transform it to an absolute one
• Passing the branch's name in the URI might be usefull to (the script could then use it to determine the absolute path)

Note: this is definitely possible -- see the `xdebug.file_link_format` option for Xdebug on http://xdebug.org/docs/stack_trace , for instance.

No branch

Hello,

I have installed and configured Crew from master (7eb548a).
I added some projets from github and these are no problems. Barre repo is present on filesystem.

But I can not see branches. All my projets have no branch in Crew, however branches really exists.
In MySQL, branch table is still empty.

I don't find a way to add branches.

Any ideas ? tips ?

Best regards,
Olivier.

unsable toolbar when review a long filename

unsable toolbar when review a long filename (cf screenshot)

public API?

Hello, I'm little bit puzzled about authentication system of application. For review you need to provide some credentials and log in, but the API is completely public. In other words, anyone can (without any other settings) read for example the list of the repos.

Does this mean that the app should be run only in closed networks?

Comment for a "block", and not only a "line"

Quite often, a comment goes with a block of code, and not a single line.

With the current version, the reviewer has to choose 1 line on which the comment will be assigned ; and the reviewee sees the comment in the middle of the code block, at the corresponding line.

When there are several comments on a file, each comment corresponding to a block of code (more than a single line), it's not easy to understand which comment corresponds to which lines.

It would be useful if a comment could be assigned to a block of code (from line X to line Y), and not just to a single line.

Add 'configuration' page to admin

Simple CRUD for 'configuration' table

useless quotes around last commit message

quotes are displayed around the last commit message on the branch list.

they are useless and could be removed.

Highlight new comments on a file

When a file is invalidated, it's usual that the developer answers a comment.
Providing that you don't always have an active notification system (by email, jabber or whatever), tt might be an interesting feature to have a visual signal to highlight this information.

I can see this implemented in 2 different ways :
1- add another state to say "hey, please, check my comment", that has to be set manually
2- provide a new functionnality on the file list that highlights the files that have been changed since your last visit

When clicking on the comment bubble in file list, go dirrectly to first comment

When clicking on the comment bubble in file list, go dirrectly to first comment.

Can be achieved with an anchor.

Add IRC notifier

This is a feature request that I can work on it.

To connect to an irc channel and post a message, it could use :

• https://github.com/phergie/phergie-irc-connection
• https://github.com/phergie/phergie-irc-generator

** sorry, wrong project **

And obviously I have posted this issue to the wrong project, sorry guys ;)

Link to user infos in admin section

The link to user infos in admin section is not well formated, it should be :

index.php/user/view/id/id

And it's:

admin.php/user/view/id/id

CrossappLink or absolute link should be fine.

Add checkbox to mask binary files in file list

In http://crew/index.php/default/fileList?branch=99

When the diff contains a lot of binary files, it's not easy to find files that could be reviewed, so a checkbox to mask them could be helpfull. (It should an option on the list, because it's a part of the review to verify them, so, they should be displayed but no always).

Change error 500 page

the error 500 page is still the symfony's default :

symfony PHP Framework
page not found
Oops! An Error Occurred
The server returned a "500 Internal Server Error".

Something is broken
    Please e-mail us at [email] and let us know what you were doing when this error occurred. We will fix it as soon as possible. Sorry for any inconvenience caused.
What's next

        Back to previous page
        Go to Homepage

Show the list of all of the commits that modified a file

A good use guide?

I've installed the project, and i've been able to clone a repo from my Github account. Now, when i commit and change/add/delete files, i go to the project, click on sync but nothing happens.

Any idea of what i'm doing grong?

non-config code shouldn't be setting php ini settings

i have to use

-    ini_set('memory_limit', '64M');
+    ini_set('memory_limit', '256M');

to make it work else i get the memory_limit error

Checking "done" on a comment changes the date of the comment

I have a file that has been reviewed ; and a comment has been written.
The header of that comment looks like this :

Author 24/07/2012 15h35 - Permalink - Done [ ]

If I click on "Done", this same header becomes :

Author 24/07/2012 15h35 - Permalink - Done by Dev 25/07/2012 10:52 [x]

Which is OK : there is a date for the comment, and a date for the fixing of the comment.

But, if I reload the page, the header is now displayed as :

Author 25/07/2012 10h52 - Permalink - Done by Dev 25/07/2012 10:52 [x]

The fixing-date is displayed as fixing-date, but also as the comment date.

The comment date should not change, ever ; and it should remain the date of the original comment, not becoming the date of the done-action.

Add an option to wrap lines

Sometimes we work with documentation files like rst or markdown.

In that cas, when the line is long, it's usefull to wrap lines.

This could be done by adding a checkbox on the left of the view file link

When checked it will add the following css properties to the pre

 white-space: pre-wrap;
 white-space: -moz-pre-wrap;
 white-space: -pre-wrap;
 white-space: -o-pre-wrap;
 word-wrap: break-word; 

and this one the the .list_body.data table .line_numbers (to display line numbers on the first line).

vertical-align-align:top;

Minimap for diff pages

When reviewing changes for very long files, it can be hard to find the actual changes - you have to scroll down quickly and spot the green / red lines as they pass.

Could something like this:
https://github.com/samcroft/mini-map
be implemented to give an overview of where the changes are in the file?

Allow Markdown syntax for comments

The title says it all : it would be nice to have Markdown syntax for comments.

Add a max number of files to review

4096 sounds good ;-)

Trim line break in notification messages (from comments)

Permalink doesn't work on new comments

• comment a line
• clic on "permalink"
• the is a 500 error

if we refresh, it works.