Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes: Annotations in Traits are ignored #240 #241

Closed

Conversation

ribeirobreno
Copy link

@ribeirobreno ribeirobreno commented Sep 23, 2021

Reverts PR #237 to fix issue #240
This bug fix will be compatible with other versions of 4.3.*.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Sep 23, 2021

We'd like to be able to turn on the new behavior via a static class method that sets a protected static boolean. We'll default the boolean to "no trait optimization" in 4.x, default it to "with trait optimization" in 5.x (main), and in the future consider whether commands in traits is something we want to continue to support going forward.

@codecov
Copy link

codecov bot commented Sep 25, 2021

Codecov Report

Merging #241 (8b4d9c0) into main (f703102) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #241      +/-   ##
============================================
- Coverage     89.77%   89.76%   -0.01%     
+ Complexity      733      731       -2     
============================================
  Files            45       45              
  Lines          1897     1896       -1     
============================================
- Hits           1703     1702       -1     
  Misses          194      194              
Impacted Files Coverage Δ
src/AnnotatedCommandFactory.php 96.35% <100.00%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f703102...8b4d9c0. Read the comment docs.

@weitzman
Copy link
Member

I would prefer to default to optimized in 4.4 (minor version). Use the flag to satisfy folks that must use traits and must upgrade.

@greg-1-anderson
Copy link
Member

I think it's easier for an app like Drush to turn the flag on than for Robo scripts to have to turn it off to maintain b/c

@greg-1-anderson
Copy link
Member

Because I already merged changed for 4.4.0 onto the 4.x branch, I released 4.3.3 as a branch off of the 4.3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants