Luke Carbis
lukecarbis

Aug 24, 2016
lukecarbis opened issue xwp/stream#871
@lukecarbis
Filter by User Role
Aug 24, 2016
lukecarbis closed issue xwp/stream#836
@lukecarbis
Custom Roles not tracking
Aug 24, 2016
lukecarbis closed issue xwp/stream#852
@lukecarbis
Exclude Rules interface not working after Select2 update
Aug 24, 2016
lukecarbis closed issue xwp/stream#866
@lukecarbis
Updating plugin just fataled my site
Aug 24, 2016
lukecarbis commented on issue xwp/stream#858
@lukecarbis

Hi wrongware - I'm not sure I see the action which you're referring to. Could you please send a pull request as an example?

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

@faishal This is looking really strong. I've made a whole bunch of comments, including some really small spacing changes (sorry about that). There'…

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Should the Query class be merged into this class?

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Should the Query class be merged into this class?

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

We should change the query, where it's defined above, to: $query = "SELECT SQL_CALC_FOUND_ROWS %1$s FROM %2$s %3$s WHERE 1=1 %4$s %5$s %6$s"; And…

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

The query isn't prepared - also, it could be filtered.

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Don't think we need this line.

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Good thinking.

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Can't use a variable as the first arg of esc_html__(). Instead: if ( ! $this->db ) { $error = __( 'Stream: Could not load chosen DB driver.', 'str…

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

I think we need some extra spacing around the brackets here: $vals = ( is_array( $vals ) && 0 !== key( $vals ) ) ? array( $vals ) : $vals;

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Extra line here can be removed.

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Too much spacing after each of these properties. They can be aligned with less spaces.

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#865
Aug 19, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

I don't think we need to define the element param here. var updateFilterActions = function() {

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

Can this be alternately expressed as: select2_data[0].data( 'group' )? Or maybe: select2_data[0].attr( 'data-group' )?

Aug 19, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

Use spaces, not tabs, when aligning variables like this. var success = x, actions = y;

Aug 18, 2016
lukecarbis commented on pull request xwp/stream#865
@lukecarbis

Will test it today, but can you fix the unit test that's failing?

Aug 18, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

@jacobschweitzer Looking good. I've made a few changes. Could you please review, and leave your thoughts? If you're happy, we can merge, and start …

Aug 18, 2016
lukecarbis commented on pull request xwp/stream#844
@lukecarbis

This is looking great! Only two things I found worth bringing up: I feel that the IFTTT connector might need some instructions / extra setup infor…

Aug 18, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

I still think it's the best way forward, though.

Aug 18, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

This will mean that, unfortunately, third-party extension initialising classes without the class name as the key will not get the benefit of auto-f…

Aug 18, 2016
@lukecarbis
Aug 18, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

This won't work for multiname connectors, such as Connector_User_Switching, and it won't work if the connector's name property doesn't match it's c…

Aug 18, 2016
@lukecarbis
  • @lukecarbis ad5651d
    Use wp_json_send_success, other minor context aware actions changes
Aug 18, 2016
lukecarbis commented on pull request xwp/stream#869
@lukecarbis

Can we use wp_send_json_success() instead here, and drop the die()? The js would obviously need to be updated to support this, too.