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

[FrameworkBundle] Adding the extension XML #22676

Closed
wants to merge 1 commit into from

Conversation

@flug
Copy link
Contributor

flug commented May 9, 2017

Q A
Branch? <3.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no

XmlUtils

This pull request is referenced on symfony/skeleton

@Pierstoval

This comment has been minimized.

Copy link
Contributor

Pierstoval commented May 9, 2017

PR name should be prepended with [FrameworkBundle], for the rest I'm 👍

@flug

This comment has been minimized.

Copy link
Contributor Author

flug commented May 9, 2017

Oh right, I redid this pull request on FrameworkBundle symfony/framework-bundle#15

@Pierstoval

This comment has been minimized.

Copy link
Contributor

Pierstoval commented May 9, 2017

Don't make PR on symfony/framework-bundle#15, this is a read-only split of the full-stack repo.

This #22676 PR is good, you just have to rename the PR and add [FrameworkBundle] at the beginning of it so it can be tagged properly and fabbot.io doesn't complain 🙂

@flug flug force-pushed the flug:add-xml-extension branch from f4d05f5 to cdc0222 May 9, 2017
required by XmlUtils and not installed by default
@flug flug changed the title Adding the extension XML for required by XmlUtils and not installed by default [FrameworkBundle] Adding the extension XML May 9, 2017
@flug flug force-pushed the flug:add-xml-extension branch from cdc0222 to 3cb6b83 May 9, 2017
@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented May 9, 2017

Thank you @flug.

fabpot added a commit that referenced this pull request May 9, 2017
This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #22676).

Discussion
----------

[FrameworkBundle] Adding the extension XML

| Q             | A
| ------------- | ---
| Branch?       | <3.3
| Bug fix?      | yes
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->

![XmlUtils](https://cloud.githubusercontent.com/assets/1810304/25841740/9b655036-34a1-11e7-9d1e-a23928b8ed17.png)

This pull request is referenced on ![symfony/skeleton](symfony/skeleton#7)
<!--
- Bug fixes must be submitted against the lowest branch where they apply
  (lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the master branch.
- Please fill in this template according to the PR you're about to submit.
- Replace this comment by a description of what your PR is solving.
-->

Commits
-------

6fe2ad0 [FrameworkBundle] Adding the extension XML
@fabpot fabpot closed this May 9, 2017
@xabbuh

This comment has been minimized.

Copy link
Member

xabbuh commented May 10, 2017

I am not sure about this change. We did not explicitly require other PHP standard extensions in the past, did we? And if we were to do this for the FrameworkBundle, we should IMO do the same for the other bundles too.

@fabpot fabpot mentioned this pull request May 17, 2017
This was referenced May 29, 2017
fabpot added a commit that referenced this pull request Jul 6, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

require the XML PHP extension

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22676 (comment), symfony/symfony-standard#1099
| License       | MIT
| Doc PR        |

I suggest to either revert #22676 or to be consistent and require the XML extension in all bundles as well as in the `symfony/symfony` package.

Commits
-------

032e654 require the XML PHP extension
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.