Opened 3 years ago

Closed 3 years ago

#5873 closed defect (bug) (maybelater)

Meaningless query

Reported by: sunxiyuan's profile sunxiyuan Owned by:
Milestone: Priority: normal
Component: Translate Site & Plugins Keywords: close


file: 85)


$gp_project = GP::$project->by_path( "wp-plugins/$slug" );
if ( ! $gp_project ) {
    WP_CLI::error( 'Invalid plugin slug.' );

$stable_tag = $this->get_plugin_stable_tag( $slug );
$branch = ( 'trunk' !== $stable_tag ) ? 'stable' : 'dev';

$gp_project = GP::$project->by_path( "wp-plugins/$slug/$branch" );
if ( ! $gp_project ) {
    WP_CLI::error( 'Invalid plugin branch.' );

The value of the first $gp_project is overwritten by the second query, so I think the first query is meaningless

Or we should only execute the second query when the value is not obtained the first time

Change History (3)

#1 @SergeyBiryukov
3 years ago

  • Component changed from Site to Translate Site & Plugins

#2 @SergeyBiryukov
3 years ago

  • Keywords close added

Hi there, welcome to WordPress Meta Trac! Thanks for the ticket.

It looks like the first query is used to check whether the plugin slug exists, and the second one to check whether the plugin branch exists for that slug.

I don't think we can do both at the same time, as in that case, if either the slug or the branch does not exist, we won't be able to tell which one is the problem.

So I think the current code is correct and works as expected.

#3 @dd32
3 years ago

  • Resolution set to maybelater
  • Status changed from new to closed

While these could be combined, get_plugin_stable_tag() is best to be avoided for non-existing slugs, so the existing code while seemingly redundant, is reasonable.

Thanks for raising this @sunxiyuan - if this was in a more highly queried endpoint it would make sense to optimise it, but as it's only within a cron task it's better to be safe and include the extra checks.

Note: See TracTickets for help on using tickets.