Skip to content

Conversation

@tarecord
Copy link
Contributor

@tarecord tarecord commented Jan 5, 2022

Fixes some phpcs issues when generating a taxonomy with so wp s1 generate tax

@tarecord tarecord requested a review from defunctl January 5, 2022 14:44
@tarecord tarecord self-assigned this Jan 5, 2022
@tarecord
Copy link
Contributor Author

tarecord commented Jan 5, 2022

After working with this on a project, I found that adding type hints to a class extending Taxonomy_Config causes errors to be thrown. Are there any blockers to adding the proper type hints in Taxonomy_Config?

/**
* @var string[]
*/
protected array $post_types = [ %5$s ];
Copy link
Contributor

Choose a reason for hiding this comment

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

The max version for tribe libs at the moment is PHP 7.2 so this should not include property types (only commented)

class Subscriber extends Taxonomy_Subscriber {
protected $config_class = Config::class;

protected string $config_class = Config::class;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@defunctl
Copy link
Contributor

defunctl commented Jan 5, 2022

After working with this on a project, I found that adding type hints to a class extending Taxonomy_Config causes errors to be thrown. Are there any blockers to adding the proper type hints in Taxonomy_Config?

Yes, we would need to release a major version update for tribe libs where we update the entire Monorepo to PHP 7.4+, which definitely needs to done.

@defunctl
Copy link
Contributor

defunctl commented Jan 5, 2022

@tarecord what we should do for now is update the generators to have the proper PHPCS ignores, and then later remove them once they are PHP7.4+ compatible. The PHPCS v2 PR on SquareOne has some examples of this:

  1. https://github.com/moderntribe/square-one/blob/8828fcd694c3192a889b5481d7feca90738ad233/wp-content/plugins/core/src/Taxonomies/Example/Config.php#L8
  2. https://github.com/moderntribe/square-one/blob/8828fcd694c3192a889b5481d7feca90738ad233/wp-content/plugins/core/src/Taxonomies/Example/Subscriber.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants