Skip to content

Added standard bundle installation instructions #4163

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

Closed
wants to merge 11 commits into from
Closed

Added standard bundle installation instructions #4163

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b297d29
First draft of the bundle installation instructions
javiereguiluz Aug 22, 2014
11db860
Applied all the fixes and suggestions made by reviewers
javiereguiluz Aug 22, 2014
21ce2c0
Fixed minor typo
javiereguiluz Aug 22, 2014
ed96447
More improvements based on reviewers' comments
javiereguiluz Aug 22, 2014
c39f7d3
Fixed some code formatting
javiereguiluz Aug 22, 2014
322ec60
Removed the first person perspective and other minor fixes
javiereguiluz Aug 22, 2014
4b29de2
More tweaks and fixes
javiereguiluz Aug 22, 2014
41ce2a5
Minor fixes and tweaks
javiereguiluz Aug 23, 2014
7a6644b
Removed the sidebar and made some minor tweaks
javiereguiluz Aug 23, 2014
35dddba
Reworded a bit the installation instructions
javiereguiluz Aug 23, 2014
7097070
Added the fixes suggested by Ryan
javiereguiluz Aug 27, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions cookbook/bundles/best_practices.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,56 @@ Extensive documentation should also be provided in the
the ``Resources/doc/`` directory; the ``Resources/doc/index.rst`` file is
the only mandatory file and must be the entry point for the documentation.

Installation instructions
Copy link
Member

Choose a reason for hiding this comment

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

"Instructions" should be upper-cased.

~~~~~~~~~~~~~~~~~~~~~~~~~

In order to ease the installation process of third-party bundles, we propose
you to use the following standardized instructions in your ``README`` file.

.. sidebar:: Installation Instructions Template
Copy link
Member

Choose a reason for hiding this comment

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

you should either add a note saying this is a rST template, or give also a Markdown template. Most projects are using Markdown for the readme on github.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. RST is the official format for the bundle documentation, but 99% of bundles hosted on GitHub use README.md instead of README.rst. I've switched the template to Markdown.

Copy link
Member

Choose a reason for hiding this comment

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

well, the best practice in the doc was written at a time where symfony.com was expected to have a repository of bundles with their doc being rendered (KnpBundles appeared as a third-party solution to this problem because the official tool announced by Fabien was not yet there, and never became a reality given we then choose composer meaning that KnpBundles and Packagist can probably be enough and people can use libraries which are not bundles). The de-facto standard then became Markdown because everyone is using Github (well, mostly at least) and it used to suck at rendering rST (3 years ago when building the Symfony2 ecosystem, the code-block directive was not supported by their rST renderer for instance) while Markdown was well rendered

Copy link
Member Author

Choose a reason for hiding this comment

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

To make myself more clear, this is what I would like to have:

doc_bundles

Copy link
Member

Choose a reason for hiding this comment

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

but ..raw does not achieve this. It let you pass unprocessed context (i.e. either html or latex or other format, which is why it asks you for a format btw: you have to provide it for each output format).
What you expect in your image is still a <pre> tag in HTML, and it is technically a code-block (so <pre><code> is more semantic).
the only difference with the rendering of ..code-block is that you don't want line numbers (this is fine, Sphinx has an option to omit them in its directive) and a different background color (and this is a matter of theming) But I could totally live with the code being on a block background, and people will be able to copy-paste it.
See http://symfony.com/doc/current/contributing/code/patches.html#make-a-pull-request and you will see that the code snippet containing the pull-request template can totally be copy-pasted (we did not even bothered about removing line numbers on it)

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right! We just need a simple .. code-block:: text directive to output the Markdown content "as is".

Copy link
Member

Choose a reason for hiding this comment

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

I find this sidebar really strange

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree! It's strange form the point of view of documentation source ... but I think it looks OK from the point of view of the documentation reader: see a screenshot of the result If you still think that this is wrong, I'll change it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think it's wrong. A sidebar shows something that is not the main content, but just a little "side information". Things that are good to know, etc. (the "How is this event used in the Symfony Framework" sidebars of the HttpKernel docs are a good example of this).

In this case, it is the most important information of the complete section.

Copy link
Member Author

Choose a reason for hiding this comment

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

The sidebar is now gone.


.. raw:: rst
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't ..code-block:: rst be better ? It is a snippet which should be rendered as such.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. The thing is that we want bundle creators to see this text "as is", without being rendered. That way they can copy-and-paste those instructions to their README.md files. Considering all this, I think that only the raw directive allow us to do this.

Copy link
Member

Choose a reason for hiding this comment

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

Well, I think you can copy-paste code-blocks directly, no ? It shoud look like a code-block containing markdown in the website, not as raw text in the middle of the HTML (which would look awful).

..raw is about providing the HTML content of the page directly. This is not what you want here. What you want is a snippet of rST or Markdown code


Installation
============

Step 1: Download the Bundle
---------------------------

Use Composer to add the bundle as a new dependency of your project:

.. code-block:: bash

$ composer require "<vendor>/<bundle-name>:dev-master"
Copy link
Member

Choose a reason for hiding this comment

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

rather than <vendor>/<bundle-name>, I would use a single placeholder <package-name>

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree! Let's use <package-name> everywhere.


This command requires to have installed Composer globally, as explained
`in the installation chapter <https://getcomposer.org/doc/00-intro.md>`_
of Composer documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't be "have Composer installed globally"?

Copy link
Member Author

Choose a reason for hiding this comment

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

And we could also use "to have Composer globally installed". I'm not sure what to do. We need the help of some native speaker.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @raulfraile. I would also remove everything after the comma and directly link "Composer globally installed".


Step 2: Enable the Bundle
-------------------------

Then, enable the bundle adding the following line in the ``AppKernel``
class of your project:

.. code-block:: php

<?php
// app/AppKernel.php

Copy link
Member

Choose a reason for hiding this comment

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

We can add a // ... here. What do you think?

public function registerBundles()
{
$bundles = array(
// ...

Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this blank line.

new <vendor>\<bundle-name>\<bundle-long-name>(),
);
}

(optionally add *Step 3*, *Step 4*, etc. to explain other required
Copy link
Member

Choose a reason for hiding this comment

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

This sentence should be outside the ..raw block

installation steps, such as registering routes or dumping assets)


Routing
-------

Expand Down