Skip to content

Add views example to MicroKernelTrait description #7568

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 7, 2017
Merged

Add views example to MicroKernelTrait description #7568

merged 1 commit into from
Mar 7, 2017

Conversation

dominikhajduk
Copy link
Contributor

PR after issue #7348 reported by @olberger.
To make Building your own Framework with the MicroKernelTrait fully described mentioned base.html.twig and micro/random.html.twig should be presented.

@dominikhajduk dominikhajduk mentioned this pull request Mar 4, 2017
Closed
@dominikhajduk dominikhajduk force-pushed the patch-2 branch 5 times, most recently from a7b3c40 to 1bf5c1b Compare March 4, 2017 22:17
javiereguiluz
javiereguiluz reviewed Mar 6, 2017
View reviewed changes
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.

What if we don't display a full Twig inheritance example and just use a simple and minimal HTML template for micro/random.html.twig?

@dominikhajduk
Copy link
Contributor Author

@javiereguiluz thanks for comment. base.html.twig is mentioned in project structure. That's why I added them. But I can also reduce changes to simple: micro/random.html.twig:

.. code-block:: html+twig

    <!DOCTYPE html>
        <html>
        <head>
            <meta charset="UTF-8" />
            <title>Random action</title>
        </head>
        <body>
            <p>{{ number }}</p>
        </body>
    </html>

Should I leave just micro/random.html.twig in this way?

@javiereguiluz
Copy link
Member

@dominikhajduk that was my idea ... but looking at it I don't like it 😐

Let's ping @HeahDude, @wouterj and @xabbuh to ask their opinion. Options:

  1. Show both the "micro" template and the "base" template
  2. Remove "base" and create a full "micro" template
  3. Show "micro" but don't show "base" (assume that the reader understands the "micro" extends from "base" and you need to create that somehow)

@HeahDude
Copy link
Contributor

HeahDude commented Mar 6, 2017

Hello there, here some thoughts:

  1. Ok, but looks too verbose.

  2. The best choice IMO, because of other reasons.

  3. I would say it's a no-go, let's not assume anything, or at least mention and link how template inheritance works.

@dominikhajduk dominikhajduk force-pushed the patch-2 branch 2 times, most recently from 1b3e8ac to e634637 Compare March 7, 2017 07:34
@dominikhajduk
Copy link
Contributor Author

Thanks @HeahDude. According to both of you comments I removed "base" and create a full "micro" template.

@wouterj
Copy link
Member

wouterj commented Mar 7, 2017

I would say: Remove base.html.twig from the file structure and keep the code like it is now. Everyone with some experience knows it can use template inheritance, everyone without experience will read the Templating guide and discover they can use template inheritance. This is really not more than a quick showcase, so not using best practices 100% isn't important imo.

@dominikhajduk
Copy link
Contributor Author

@wouterj thanks for comment. You're right that template inheritance is not a key for that page. Removed base.html.twig for file structure and leaved just minimum template.

@wouterj
Copy link
Member

wouterj commented Mar 7, 2017

👍 Looks perfect!
status: reviewed

@xabbuh xabbuh merged commit 669b29e into symfony:2.8 Mar 7, 2017
xabbuh added a commit that referenced this pull request Mar 7, 2017
@xabbuh
minor #7568 Add views example to MicroKernelTrait description (domini…
b058d96 
…khajduk)

This PR was merged into the 2.8 branch.

Discussion
----------

Add views example to MicroKernelTrait description

PR after [issue #7348](#7348) reported by @olberger.
To make [Building your own Framework with the MicroKernelTrait](http://symfony.com/doc/current/configuration/micro_kernel_trait.html) fully described mentioned `base.html.twig` and `micro/random.html.twig` should be presented.

Commits
-------

669b29e Add example views to MicroKernelTrait description
xabbuh added a commit that referenced this pull request Mar 7, 2017
@xabbuh
[#7568] minor tweaks
2d7ea6b
@xabbuh
Copy link
Member

xabbuh commented Mar 7, 2017

Thank you for these nice improvements @dominikhajduk. I have merged them and made some very minor tweaks in 2d7ea6b.

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

@javiereguiluz javiereguiluz javiereguiluz left review comments

Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dominikhajduk @javiereguiluz @HeahDude @wouterj @xabbuh @carsonbot