Code review comment for lp:~ywwg/mixxx/features_xwax2

Revision history for this message
Albert Santoni (gamegod) wrote :

I also approve. The vinyl control logic is fairly straightforward, it
just needs loads of testing now to make sure it's tight. Some final
comments:
- I would squash those qDebugs in vinylcontrolxwax.cpp before the release.
- Maybe factor some of those constants out too.
- We should make sure a tester verifies that vinyl control still works
with a 96000 Hz samplerate.

Otherwise, great stuff! Launchpad's code review interface is broken
right now so I can't approve it properly, but RJ, feel free to merge
into trunk whenever.

Thanks Owen!
Albert

On Fri, Apr 8, 2011 at 11:21 PM, RJ Ryan <email address hidden> wrote:
> Review: Approve
> OK looks like all my complaints are now fixed :) (minus VinylControlControl in EngineBuffer -- I'll remove that myself post merge) it's now up to albert to OK the vinylcontrol* classes and then we can merge this sucker.
>
> Just to get it in writing somewhere, ywwg and I talked about how I asked him to use WDisplay instead of WStatusLight, but it turns out WDisplay is geared towards being a knob but with no input. The control connected to it is scaled to 0.0 through 128.0 based on its range. This isn't suitable for indicator controls that represent fixed status values.
>
> After Owen's changes, WStatusLight will be to WPushButton what WDisplay is to WKnob.
> --
> https://code.launchpad.net/~ywwg/mixxx/features_xwax2/+merge/37798
> You are reviewing the proposed merge of lp:~ywwg/mixxx/features_xwax2 into lp:mixxx.
>

« Back to merge proposal