ooyala / barkeep Goto Github PK
View Code? Open in Web Editor NEWThe friendly code review system.
Home Page: getbarkeep.org
The friendly code review system.
Home Page: getbarkeep.org
When a commit sha is included in a comment, its automatically converted to a link to the commit. Right now, the link is broken.
eg.
http://barkeep.sv2/commits//3003df144bbd30e630941fbc6232b3309392cf79
Looks like the repo part is missing, should we assume its always the same repo as the comment is made for?
When pulling in new commits, we should send emails when a commit is ingested. Matching these commits to saved searches will be expensive, and so should be a separate background job, not part of the commit importer.
Let's make it go away.
We are currently using the authored date for commit ingestion (so that's what is stored in the DB), but the commit date for searching. An example is the commit 45f31c9027782db69c6b76c5a04a1e833437891c in the backlot repo -- it will show as '6 hours ago' in the saved search but '4 days ago' in the commit view (as of writing this commit).
We definitely need to use the commit date for searching (otherwise newly pushed commits won't appear at the top of the list), so my vote is that we use commit date everywhere or store both dates and display them both in the commit metadata section.
We also need to think about how we will fix production data in either case.
This is because the browser is caching the page as it's first rendered, not at its last-known-state. It's a bewildering UX.
The emails especially would make watching commits by a list of people far less error prone. You don't have to precisely specify their email address if it's autocompleted.
Right now, we don't specify that MySQL should use InnoDB tables, so with pre-5.5.5 MySQLs we'll get MyISAM tables. This means that our foreign constraints and transactions aren't enforced.
We need to specify
Sequel::MySQL.default_engine = 'InnoDB'
somewhere in the configuration. We also need to dump our prod DB, re-run the migrations from scratch, and restore the data (to make the switch to InnoDB).
I suspect you're limited by git here, but a query of authors:y results in any commits by people with a 'y' in their name. I suspect you'd have to implement your own filtering to solve this, rather than depending on git queries.
This will help you spot commits under discussion. In the case where you're trying to get your list down to zero because you're filtering by unapproved commtis only, if there's a commit that's sitting there unapproved and has a bunch of comments next to it, this will help you remember that it is a work in progress.
We should not show the comment indicator next to the commit if there are no comments, and we should make the indicator really low profile, e.g. [num][32px comment icon]
This is because we blow-away the google session IDs stored in barkeep/tmp. The deploy shouldn't push these, for one, and they should be preserved across deploys.
I'm missing at least one commit from my view in the Ooyala Barkeep installation. Commit hash is 6993992207fd14d70ddc59f4119ad288702ebaab . Search is authors:noah
This has several advantages:
The current pie chart is useless because not all commits are meant to be reviewed (i.e. the ones by people using a different CR system). A better way could be to only count commits by people who are in a list of "active users".
Also, it would be great to be able to specify a time range for the pie chart.
I'll take this.
Heard some grumbling the other day from people who didn't know why they got emails from barkeep. I think it'd be helpful if there were a little blurb in each email saying why they received it: because it was in a saved search, they commented on it, etc.
I watched both Sean and Nimrod use this feature the other day. They both wanted to see more context, and I showed them the hidden keyboard shortcut for it (we should add a non-keybroad driven UI for context as well). When they hit the shortcut to show the entire file, thousands of lines suddenly appeared and it was then hard to navigate between the diff chunks.
We should instead add a UI and shortcuts to increase or decrease the context by hunks of 10 lines or so.
This is a minor but bizarre bug. In commit files with only a few lines, the table cell widths are much more widely spaced than files with lots of lines. However, the cells do expand to fit when there are lots of lines (see the wtf.git repo for an example of a file with about 18k lines).
Anyway, worth checking out when we've fixed a bunch of our high-priority bugs.
In the single commit view, we should really display some kind of indicator between chunks instead of just having discontinuous line numbers. This can be kind of confusing.
The 'low-hanging fruit' approach is probably just to display a thick bottom border on the last tr
of each chunk in the compressed diff view.
There should be a text field that allows a committer to request a review by a specific person. The text field should auto-expand the email address like gmail does.
The main difficulty is to show them some UX to verify that the commit has been approved -- maybe scrolling to the button, or sliding up a message from the button of the window saying "approved" ala Vimium.
Hitting this key twice should not toggle it again from approved to unapproved.
Similar to #1. It would be nice to avoid sliding the whole file over (it looks bad because the top part with the filename doesn't align with anything when it slides).
This is difficult to do since on the client, we don't know if new commits have yet arrived.
adding a new comment when fenced code blocks in existing comments are on the page nests comment forms in existing comment forms.
We have two such failed emails as of Aug 31: http://barkeep.sv2/admin
Seems to skip way down the page, not actually between chunks.
For the search:
authors:x,y,z
I get title:
Commits by x, y, z, and z on master
I expect to get:
Commits by x, y, and z on master
This is a low-priority cosmetic enhancement, but it would be cool. We could also the 110 value user-customizable.
On my Mac OS X Lion machine using the latest ruby 1.9 and latest ruby-openid, when I try to run the server I get a nasty crash (bus error). Here's the beginning of the error message:
/Users/caleb/.rubies/1.9.2-p290/lib/ruby/gems/1.9.1/gems/ruby-openid-2.1.8/lib/openid/dh.rb:60: [BUG] Bus Error
My temporary workaround is to hack my copy according to the patch here. We should investigate this more thoroughly and figure out a better way to solve this issue. Maybe there are alternatives to this gem?
It would be cool to get rid of this. It's more apparent on large monitors and when you scroll.
Evan complains that paging through saved searches is too slow. It may be that the call to get the next page of commits is slow, or the animation is too slow. We should see which of those two operations is "finishing first" and work on making the longest of them shorter.
Scroll right on a saved search - the page number remains 1.
Is it possible to have a count of the total number of pages? Page 1/5 rather than just Page 1.
Open to debate, but I think it would be helpful. See this commit:
http://barkeep.sv2/commits/backlot/4662969f76899954e49bce183159a76a71f86e7c
It has really long lines because it's an ERB file which was edited with line wrap on.
I have received no emails about new commits, even though new commits show up in my saved search on barkeep.sv2. I do receive code review emails.
Multiple folks have asked for this and it's pretty critical.
We should also rename the checkbox to "show unapproved".
Right now, it has to be toggled on for every commit the user goes to.
Barkeep says commits which are 44 hours old happened "a day ago." To me, that's 2 days ago. Perhaps it should round anything >= 1.5 days and < 2.5 days to be 2 days.
I had great trouble building a search for "Al"; I eventually realized that the string "al" matches any @ooyala.com email address.
The only way to overcome this now seems to be to find a part of their name or email address which is not a sub-string of anyone else's, if possible.
Screenshot:
There are little gaps where the chunk breaks were when you expand. I thought this was fixed in 0433678, but either that didn't work or a regression was introduced with later (maybe in side-by-side?)
Hi there. I'm not getting any emails for searches in which I've checked "email me new commits." search is branches:prodtools/logging-dev/2011-08-31 . One commit which appears in the search but which I didn't get is ad23879d6416c16d1638dbfce08ed86504e8a2fb
From Evan:
I’m not getting threaded e-mails for multiple comments by different people; e.g., e50500d --edanaher... um... for 8808e7f, I have two threads (for me and Sid), but my most recent comment was in the thread that was otherwise Sid. -- edanaher
Animating too many lines becomes a slide show anyway, should just take the user straight to end state in that case.
Instead of scrolling through all the code-review emails, can I create a saved-search to find all my commits that have been commented on by somebody.
After I've answered/fixed the feedback in that commit, is there a way to make sure I don't see that commit again?
Basically, I'd only like to see commits for which there is an action item.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.