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] Graphviz dumper escape not always a string #30102

Merged
merged 1 commit into from Feb 8, 2019

Conversation

Projects
None yet
5 participants
@Korbeil
Copy link
Contributor

Korbeil commented Feb 7, 2019

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

The escape function takes a string as parameter but it can be a bool (see findTransitions method in same class). In this PR I allow any type in escape and does escaping only for string.

@lyrixx lyrixx changed the base branch from 4.1 to 4.2 Feb 7, 2019

@lyrixx

This comment has been minimized.

Copy link
Member

lyrixx commented Feb 7, 2019

I have update the base branch as it should target 4.2. Could you rebase please?

@Korbeil Korbeil force-pushed the Korbeil:fix/workflow-graphviz-dumper branch 2 times, most recently from eb991e4 to 757b784 Feb 7, 2019

@lyrixx lyrixx force-pushed the Korbeil:fix/workflow-graphviz-dumper branch from 757b784 to 7b957ce Feb 7, 2019

@Korbeil Korbeil force-pushed the Korbeil:fix/workflow-graphviz-dumper branch from 7b957ce to 0613f7d Feb 7, 2019

@lyrixx

lyrixx approved these changes Feb 7, 2019

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Feb 7, 2019

@Korbeil Korbeil force-pushed the Korbeil:fix/workflow-graphviz-dumper branch from 0613f7d to 1b17192 Feb 7, 2019

@lyrixx

This comment has been minimized.

Copy link
Member

lyrixx commented Feb 8, 2019

Good catch, thanks Baptiste.

@lyrixx lyrixx merged commit 1b17192 into symfony:4.2 Feb 8, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details

lyrixx added a commit that referenced this pull request Feb 8, 2019

bug #30102 [Workflow] Graphviz dumper escape not always a string (Kor…
…beil)

This PR was merged into the 4.2 branch.

Discussion
----------

[Workflow] Graphviz dumper escape not always a string

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

The `escape` function takes a string as parameter but it can be a `bool` (see `findTransitions` method in same class). In this PR I allow any type in `escape` and does escaping only for `string`.

Commits
-------

1b17192 escape function does not always take a string

@Korbeil Korbeil deleted the Korbeil:fix/workflow-graphviz-dumper branch Feb 8, 2019

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