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

[Workflow] Move code from ValidateWorkflowsPass to the FrameworkExtension #29457

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@lyrixx
Member

lyrixx commented Dec 4, 2018

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Just some cleaning. See
#29146 (comment)

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class ValidateWorkflowsPass implements CompilerPassInterface

This comment has been minimized.

@xabbuh

xabbuh Dec 4, 2018

Member

Did we never register the pass inside the FrameworkBundle? Also, removing the class here is a BC break for anyone using it in a standalone setup.

This comment has been minimized.

@lyrixx

lyrixx Dec 5, 2018

Member

Indeed, I forgot to remove it from the framework bundle.
I'm not sure if we should consider it as a BC break for the reason I explained in #29146 (comment)

@lyrixx lyrixx force-pushed the lyrixx:workflow-validation branch from 8dbbde6 to 378c4f9 Dec 5, 2018

@lyrixx lyrixx force-pushed the lyrixx:workflow-validation branch from 378c4f9 to 7c4afd4 Dec 5, 2018

@nicolas-grekas nicolas-grekas added this to the next milestone Dec 6, 2018

@stof

This comment has been minimized.

Member

stof commented Dec 7, 2018

The compiler pass should be deprecated, not removed. It don't think making such BC break is actually worth it compared to the deprecation path.

@lyrixx

This comment has been minimized.

Member

lyrixx commented Dec 7, 2018

Okay, Il will deprecate it.

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