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

Deprecate XML routing file in favor of YAML #925

Merged
merged 1 commit into from
May 8, 2017

Conversation

robfrawley
Copy link
Collaborator

@robfrawley robfrawley commented May 6, 2017

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

Deprecating the XML routing file in favor of YAML enables providing a more user-friendly format for the Symfony Flex recipe, and YAML is, IMHO, generally more user-friendly for this bundle as a stand-alone package, as well.

The old routing.xml file remains, to provide backward compatibility for users, but instead of containing routes it simply imports the new routing.yaml file. The XML file is deprecation in the UPGRADE.md file and can be safely removed from the 2.x branch entirely.

The installation RST documentation has been updated to use the YAML routing varient, as well.

@lsmith77 lsmith77 added the State: Reviewing This item is being reviewed to determine if it should be accepted. label May 6, 2017
@robfrawley robfrawley self-assigned this May 6, 2017
@robfrawley robfrawley added RFC: Draft This indicates the described RFC is still being drafted. and removed State: Reviewing This item is being reviewed to determine if it should be accepted. labels May 6, 2017
@robfrawley robfrawley added this to the 1.7.5 milestone May 6, 2017
@robfrawley robfrawley mentioned this pull request May 6, 2017
methods:
- GET
requirements:
filter: '[A-z0-9_\-]*'
Copy link
Contributor

Choose a reason for hiding this comment

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

If a dash is at the last position of a character match you don't need to escape it, so you could use [A-z0-9_-]*.

<requirement key="path">.+</requirement>
</route>
</routes>
</routes>

Choose a reason for hiding this comment

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

Missing new line here :)

@@ -0,0 +1,23 @@
---

Choose a reason for hiding this comment

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

Why?

@robfrawley robfrawley force-pushed the feature-migrate-routing-to-yaml branch from cd13162 to f4c4af4 Compare May 7, 2017 16:12
@robfrawley
Copy link
Collaborator Author

@rpkamp @Pierstoval Comments addressed; thanks!

UPGRADE.md Outdated
@@ -1,5 +1,10 @@
# Upgrade

## 1.7.5

Choose a reason for hiding this comment

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

In terms of semver, deprecations should come in minor versions, not patch versions, is it a problem if you need to push these changes to 1.8.0 instead of 1.7.5?

Copy link
Collaborator Author

@robfrawley robfrawley May 7, 2017

Choose a reason for hiding this comment

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

Nope, though it looks like we've broken this rule a bunch in prior patch releases. :-(

Choose a reason for hiding this comment

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

Ok, well if it's a bugfix it's totally fine 😆

@robfrawley robfrawley force-pushed the feature-migrate-routing-to-yaml branch from f4c4af4 to 81f9856 Compare May 7, 2017 17:51
@robfrawley robfrawley merged commit 5aab778 into liip:1.0 May 8, 2017
@robfrawley robfrawley mentioned this pull request May 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC: Draft This indicates the described RFC is still being drafted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants