Skip to content

Page Creation Small Changes #10177

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 5 commits into from
Closed

Page Creation Small Changes #10177

wants to merge 5 commits into from

Conversation

ahinkle
Copy link
Contributor

@ahinkle ahinkle commented Aug 11, 2018

This PR includes changes to the page creation page for the user to easily understand the breakdown and removes unnecessary areas.

  • Move the Controller tip to the example controller area instead of the annotation route. The hint currently drops unexpectedly to the end user after a routes discussion.

  • The profiler-pack is already included with the Symfony package. Removed composer installation area. Additionally, users may be confused why there is more than one route when it declares "You should see one route" -- added a note on debugging routes.

  • Twig is automatically included with Symfony. No need to direct the user to require and install it with composer.

  • Shorter, more elegant array syntax on the render() example.

Copy link
Member

@weaverryan weaverryan 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 a lot! This page was last updated when symfony/skeleton was used. So, indeed, some of the assumptions are now wrong.

The trickiest part for me is no longer saying you should install Twig. The changes in this PR should be accepted in my opinion. However, it does make things harder for symfony/skeleton users. I think the best we can do is to add the composer require lines at the top of the specific topic guides (e.g. the templating.rst page), but not in these few getting started pages (we should assume website-skeleton all the way).

Thanks @ahinkle!

@weaverryan weaverryan added this to the 4.1 milestone Aug 18, 2018
xabbuh added a commit that referenced this pull request Aug 21, 2018
weaverryan added a commit that referenced this pull request Aug 28, 2018
* 4.1:
  {% if label is not same as(false) and required %}
  minor language tweak
  Update service_container.rst
  Fix wrong exception used in custom authentication provider example
  [Console] Replace useless usage of ContainerAwareCommand
  Shorten ternary operator
  Fix wrong usage in custom authenticator checking if password is invalid
  [Process] Fix typo in Process object initialization
  update hardcoded 5.1 version link
  Explain dump server usage outside of Symfony
  Add undocumented sortByAccessedTime, sortByChangedTime and sortByModifiedTime methods in finder component
  Update bugs.rst
  [#10200] remove trailing spaces
  action_method is missing $task variable
  [#10177] fix markup
  Clarifying when you need to or don't need to install the server bundle
  Fix bad link
  Update setup.rst
  Also requiring annotations so they can be used
Guikingone pushed a commit to Guikingone/symfony-docs that referenced this pull request Feb 12, 2019
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.

3 participants