headers
Roan Kattouw | 1 Jul 2012 01:06
Picon

Re: Barkeep code review tool

On Fri, Jun 29, 2012 at 2:20 PM, Marcin Cieslak <saper <at> saper.info> wrote:
> As seen on IRC:
>
> https://github.com/ooyala/barkeep/wiki/Comparing-Barkeep-to-other-code-review-tools
>
The most prominent feature of Barkeep mentioned on this page is that
it was built for a post-commit review workflow. Given that the reason
we moved MediaWiki to git was basically so that we could move *away*
from post-commit review, I don't think using Barkeep as-is would work.

Then again, from watching the demo video (see getbarkeep.org) it looks
like their UI is a lot better than Gerrit's, and I like features like
saved searches and most-commented-on-commits dashboards. Integrating
Barkeep or the UI/UX ideas from it with Gerrit (or vice versa --
integrating Gerrit's pre-commit review workflow support with Barkeep
-- but I think that would be harder) would be cool but I have no
concrete ideas as to how it could be done right now.

Roan
Permalink | Reply | 16 comments |
headers
Subramanya Sastry | 1 Jul 2012 01:14
Picon

Re: Barkeep code review tool


> On Fri, Jun 29, 2012 at 2:20 PM, Marcin Cieslak <saper <at> saper.info> wrote:
>> As seen on IRC:
>>
>> https://github.com/ooyala/barkeep/wiki/Comparing-Barkeep-to-other-code-review-tools
>>
> The most prominent feature of Barkeep mentioned on this page is that
> it was built for a post-commit review workflow. Given that the reason
> we moved MediaWiki to git was basically so that we could move *away*
> from post-commit review, I don't think using Barkeep as-is would work.
>
> Then again, from watching the demo video (see getbarkeep.org) it looks

I assumed it was pre-commit workflow only when I first looked at 
Barkeep, but I found this snippet on getbarkeep.org that Roan posted.

"Barkeep is unopinionated. Use it with pre-commit or post-commit 
workflows, and script tools on top of it if you like. Comes with a 
command line client and REST APIs."

Subbu.
Permalink | Reply | 1 comment |
headers
Faidon Liambotis | 1 Jul 2012 01:23
Picon

Re: Barkeep code review tool

On Sat, Jun 30, 2012 at 04:06:37PM -0700, Roan Kattouw wrote:
> On Fri, Jun 29, 2012 at 2:20 PM, Marcin Cieslak <saper <at> saper.info> wrote:
> > As seen on IRC:
> >
> > https://github.com/ooyala/barkeep/wiki/Comparing-Barkeep-to-other-code-review-tools
> >
> The most prominent feature of Barkeep mentioned on this page is that
> it was built for a post-commit review workflow. Given that the reason
> we moved MediaWiki to git was basically so that we could move *away*
> from post-commit review, I don't think using Barkeep as-is would work.

Well, in the ops puppet repo though, we very often +2 commits ourselves
and push them, instead of waiting for someone else to review/approve
them. You could argue that it's our workflow that it's wrong, but I just
think the needs for infrastructure-as-code might be different from the
needs code development has.

It's like asking for pre-execution reviews of whatever you type in a
shell prompt and we can all agree that's just crazy :) In a perfect
world we'd stage every change in VMs where we'd do local puppet commits
without reviews; then push those tested changesets into the pre-commit
review system to get into production. But we're very far from that and
being perfectionists just hurts us more on our daily work.

Having a proper post-commit review workflow would be much better than
hacking around the system and reviewing commits ourselves. It'd also
allow us to actually have post-commit reviews, something that rarely
happens right now. At least I'd do that, while currently it's a PITA to
do so.
(Continue reading)

Permalink | Reply | 13 comments |
headers
Helder . | 1 Jul 2012 02:22
Picon

Re: Inline styles trouble on the mobile site

On Thu, Jun 28, 2012 at 6:43 PM, Krinkle <krinklemail <at> gmail.com> wrote:
> Replies inline:
>
> On Jun 28, 2012, at 7:38 PM, Jon Robson wrote:
>
>> # "things rely on those inline styles whether we like it or not."
>> No... They rely on styles not //inline// styles. This is my main
>> problem with the current setup! I believe the majority of things done
>> in inline styles that are essential would be better done in
>> MediaWiki:Common.css - if we have text that is red this would be
>> better down with a class markedRed otherwise some text risks being red
>> or and other text #BE3B3B despite meaning the same thing. Most layout
>> problems on mobile can be fixed with media queries. You can't do these
>> in inline styles.
>>
>
> Hold on, we're in misunderstanding :). We agree.
>
> So, they *do* rely on inline styles. But when I said "they rely" I meant: they
> currently use them to do something that should not be removed. You won't hear me
> say we "need" inline styles (there is not a single layout best done through
> inline styles). I'm saying they are in use - right now - and fulfilling valid
> needs to the point that they can break or make an article.
>
> Obviously they should become css classes loaded through modules like
> MediaWiki:Common.css and what not. I will +1 any movement towards deprecating
> them entirely from wikitext in MediaWii core. But not for just for mobile and
> not without at least a year of migration time for live sites (possibly with an
> opt-in feature before that for users to preview articles without inline styles
> to help fixing them).
(Continue reading)

Permalink | Reply | 4 comments |
headers
Subramanya Sastry | 1 Jul 2012 02:49
Picon

Re: Barkeep code review tool

I got curious about how workflows were implemented and since the 
codebase is pretty small, I started digging around their code, but then 
found some relevant information here: 
https://github.com/ooyala/barkeep/issues/254

So, it appears that workflow is not part of barkeep itself since barkeep 
and git repos are decoupled  -- this makes the default workflow 
post-commit, and pre-commit workflows are something that have to be 
scripted externally.  Not sure what is involved there, but it is 
probably a matter of time before sample scripts / guides are up.  Also, 
possibly, this might enable different commit-review workflows for 
different groups, if something like that might be useful.

Subbu.

>
>> On Fri, Jun 29, 2012 at 2:20 PM, Marcin Cieslak <saper <at> saper.info> 
>> wrote:
>>> As seen on IRC:
>>>
>>> https://github.com/ooyala/barkeep/wiki/Comparing-Barkeep-to-other-code-review-tools 
>>>
>>>
>> The most prominent feature of Barkeep mentioned on this page is that
>> it was built for a post-commit review workflow. Given that the reason
>> we moved MediaWiki to git was basically so that we could move *away*
>> from post-commit review, I don't think using Barkeep as-is would work.
>>
>> Then again, from watching the demo video (see getbarkeep.org) it looks
>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Roan Kattouw | 1 Jul 2012 05:37
Picon

Re: Barkeep code review tool

On Sat, Jun 30, 2012 at 4:23 PM, Faidon Liambotis <faidon <at> wikimedia.org> wrote:
> Well, in the ops puppet repo though, we very often +2 commits ourselves
> and push them, instead of waiting for someone else to review/approve
> them. You could argue that it's our workflow that it's wrong, but I just
> think the needs for infrastructure-as-code might be different from the
> needs code development has.
>
> It's like asking for pre-execution reviews of whatever you type in a
> shell prompt and we can all agree that's just crazy :) In a perfect
> world we'd stage every change in VMs where we'd do local puppet commits
> without reviews; then push those tested changesets into the pre-commit
> review system to get into production. But we're very far from that and
> being perfectionists just hurts us more on our daily work.
>
> Having a proper post-commit review workflow would be much better than
> hacking around the system and reviewing commits ourselves. It'd also
> allow us to actually have post-commit reviews, something that rarely
> happens right now. At least I'd do that, while currently it's a PITA to
> do so.
>
Yes, ops essentially uses a post-commit workflow right now, and that
makes sense for them. Gerrit doesn't support post-commit review very
well so Barkeep might work better there. But other projects, such as
MediaWiki core, do use real pre-commit (or pre-merge, rather) review.

> Barkeep claims to work with both post- and pre-commit workflows,
> although the details elude me.
>
Per Subbu's message, you'd have to add support for pre-commit. Gerrit
manages the git repositories themselves, implements fine-grained
(Continue reading)

Permalink | Reply | 12 comments |
headers
Antoine Musso | 1 Jul 2012 08:53
Picon
Favicon

Re: Barkeep code review tool

Roan Kattouw wrote:
> Yes, ops essentially uses a post-commit workflow right now, and that
> makes sense for them.

ops also uses pre-commit review for non-ops people :-]

--

-- 
Antoine "hashar" Musso
Permalink | Reply | 10 comments |
headers
Roan Kattouw | 1 Jul 2012 10:16
Picon

Re: Barkeep code review tool

On Sat, Jun 30, 2012 at 11:53 PM, Antoine Musso <hashar+wmf <at> free.fr> wrote:
> Roan Kattouw wrote:
>> Yes, ops essentially uses a post-commit workflow right now, and that
>> makes sense for them.
>
> ops also uses pre-commit review for non-ops people :-]
>
Yeah, that's right. What I meant to say (and thought I had said in
some form later in that message) was that the puppet repo has
post-commit review for most changes by ops staff, and pre-commit
review for everything else (non-ops staff, volunteers, and certain
changes by ops staff in some cases).

Roan
Permalink | Reply | 9 comments |
headers
Christian Aistleitner | 1 Jul 2012 13:49
Picon
Favicon

Re: Speed up tests, make <at> group Database smarter

Hi Platonides,

On Sat, Jun 30, 2012 at 03:45:14PM +0200, Platonides wrote:
> On 30/06/12 14:24, Christian Aistleitner wrote:
> > [ Mocking the database ]
> > 
> > [...]
> > One would have to abstract database access above the SQL
> > layer (separate methods for select, insert, ...) [...]
> 
> You still need to implement some complex SQL logic.

One might think so, yes.
But as I said, one would mock /above/ the SQL layer.
For typical database operations, SQL would not even get generated in
the first place!

Consider for example code containing
  $db->insert( $param1, $param2, ... );

The mock db's insert function would compare $param1, $param2,
... against the invocations the test setup injected. If there is no
match, the test fails. If there is a match, the mock returns the
corresponding return value right away.
No generating SQL.
No call to $db->tableName.
No call to $db->makeList.
No call to $db->query.
No nothing. \o/
(Continue reading)

Permalink | Reply | 4 comments |
headers
Jeroen De Dauw | 1 Jul 2012 21:12
Picon
Gravatar

Handling deletion using the latest revision

Hey,

I am writing some code that needs to be executed on every article deletion
and which needs the latest revision of the article that was deleted. Is
there any place (hook) where I can put this without doing a db read for the
revision? And if not, is there any place where I can put this where I
actually get the revision id I need or can obtain it without writing my own
query?

Cheers

--
Jeroen De Dauw
http://www.bn2vs.com
Don't panic. Don't be evil.
--
Permalink | Reply | 1 comment |

Gmane