Skip to content

Conversation

mukeshpanchal27
Copy link
Member

Summary

Follow-up work #654 (comment)

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

mukeshpanchal27 and others added 30 commits January 18, 2024 12:25
Co-authored-by: Pascal Birchler <pascalb@google.com>
Introduce plugins folder for standalone plugins
Merge trunk into feature/modules-to-plugins
Merge trunk into feature/modules-to-plugins
@mukeshpanchal27
Copy link
Member Author

Once #975 is merged, we need to rebase this PR.

@mukeshpanchal27
Copy link
Member Author

@swissspidy @joemcgill @westonruter Could you please take another look when you have a moment?

@mukeshpanchal27
Copy link
Member Author

@joemcgill @westonruter Could you please take another look when you have a moment?

wp: 'trunk'
env:
WP_ENV_PHP_VERSION: ${{ matrix.php }}
WP_ENV_CORE: ${{ matrix.wp == 'trunk' && 'WordPress/WordPress' || format( 'https://wordpress.org/wordpress-{0}.zip', matrix.wp ) }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Why not use ternary expression here? Is it allowed?

Suggested change
WP_ENV_CORE: ${{ matrix.wp == 'trunk' && 'WordPress/WordPress' || format( 'https://wordpress.org/wordpress-{0}.zip', matrix.wp ) }}
WP_ENV_CORE: ${{ matrix.wp == 'trunk' ? 'WordPress/WordPress' : format( 'https://wordpress.org/wordpress-{0}.zip', matrix.wp ) }}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added as part of #544. If we need to update this, then we also have to update similar settings for other workflows (php-test.yml and php-test-standalone-plugins.yml).

Shall we open a follow-up PR to make these changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up PR is fine. Just seems hard to read right now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter, GitHub workflow does not allow ${{ x ? 'ifTrue' : 'ifFalse' }} ternary operator. Instead, it uses ${{ x && 'ifTrue' || 'ifFalse' }}. Therefore, we don't need to open a follow-up PR for this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants