8000 use diff code block for config changes too by xabbuh · Pull Request #8661 · symfony/symfony-docs · GitHub
[go: up one dir, main page]

Skip to content

use dif 8000 f code block for config changes too #8661

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

Closed
wants to merge 1 commit into from
Closed

Conversation

xabbuh
Copy link
Member
@xabbuh xabbuh commented Nov 15, 2017

We already do that in the PHP code block below and I think doing the same here would make it more easier to follow what you need to do.

@xabbuh xabbuh added this to the 4.0 milestone Nov 15, 2017
Copy link
Member
@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

I like this idea ... but I'm not sure about this specific case. The diff should add - to the previous route and + to the uncommented import. Showing the same route commented and uncommented is not that useful in my opinion.

@xabbuh
Copy link
Member Author
xabbuh commented Nov 15, 2017

Showing - for the previous route would be wrong as you do not have to remove existing definitions.

@xabbuh xabbuh force-pushed the routing-config-diff branch from 4b7e0ef to 156f7e7 Compare November 17, 2017 11:23
@xabbuh xabbuh added the On hold label Nov 17, 2017
@xabbuh
Copy link
Member Author
xabbuh commented Nov 17, 2017

With symfony/recipes#260 we will be able to just remove the whole example. So I suggest that we just wait for it to be merged.

@weaverryan
Copy link
Member

Closed in favor of #8726

@weaverryan weaverryan closed this Nov 21, 2017
@weaverryan weaverryan deleted the routing-config-diff branch November 21, 2017 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0