Skip to content

Reviewed the Assetic cookbook articles #5094

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 2 commits into from
Mar 22, 2015
Merged

Reviewed the Assetic cookbook articles #5094

merged 2 commits into from
Mar 22, 2015

Conversation

javiereguiluz
Copy link
Member

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets -

wouterj
wouterj reviewed Mar 19, 2015
View reviewed changes
cookbook/assetic/uglifyjs.rst
Now you can execute the ``uglifyjs`` command that lives in the ``node_modules``
directory:

.. code-block:: bash

$ ./app/Resources/node_modules/.bin/uglifyjs --help
Copy link
Member

Choose a reason for hiding this comment

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

for windows compatbility, the path should be in double quotes

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 mean like this?

$ "./app/Resources/node_modules/.bin/uglifyjs" --help

Copy link
Member

Choose a reason for hiding this comment

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

yes

2015-03-19 11:41 GMT+01:00 Javier Eguiluz notifications@github.com:

In cookbook/assetic/uglifyjs.rst
#5094 (comment):

  • $ uglifyjs --help
    +Now you can execute the uglifyjs command that lives in the node_modules
    +directory:

+.. code-block:: bash

$ ./app/Resources/node_modules/.bin/uglifyjs --help

You mean like this?

$ "./app/Resources/node_modules/.bin/uglifyjs" --help

—
Reply to this email directly or view it on GitHub
https://github.com/symfony/symfony-docs/pull/5094/files#r26743251.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks.

@wouterj
Copy link
Member

wouterj commented Mar 19, 2015

👍

xabbuh
xabbuh reviewed Mar 19, 2015
View reviewed changes
cookbook/assetic/asset_management.rst
@@ -204,7 +204,7 @@ Combining Assets
~~~~~~~~~~~~~~~~

One feature of Assetic is that it will combine many files into one. This helps
to reduce the number of HTTP requests, which is great for front end performance.
to reduce the number of HTTP requests, which is great for frontend performance.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about this change. While I agree that "frontend" looks most beautiful, it seems that "front-end" actually would be more correct (see also #4671 (comment)).

Copy link
Member

Choose a reason for hiding this comment

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

both Bower and Bootstrap (on their homepages) use front-end. That looks a bit odd to me, but I guess we should follow their lead.

@weaverryan weaverryan merged commit 22ccd76 into symfony:2.3 Mar 22, 2015
weaverryan added a commit that referenced this pull request Mar 22, 2015
@weaverryan
minor #5094 Reviewed the Assetic cookbook articles (javiereguiluz)
17a6863 
This PR was merged into the 2.3 branch.

Discussion
----------

Reviewed the Assetic cookbook articles

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | -

Commits
-------

22ccd76 Fixed a path for Windows
f418e20 Reviewed the Assetic cookbook articles
weaverryan added a commit that referenced this pull request Mar 22, 2015
@weaverryan
[#5094] Removing mkdir - it's not needed (thanks xabbuh)
20dcc3c
@weaverryan
Copy link
Member

I love having a fresh set of eyes on these - thanks! @xabbuh I did remove the mkdir at sha: 20dcc3c. You're right - these are not needed.

Cheers!

weaverryan added a commit that referenced this pull request Mar 24, 2015
@weaverryan
Merge branch '2.3' into 2.6
b91f05d 
* 2.3: (44 commits)
  Removed duplicate "long"s
  terminate file with newline
  move title back and move description to separate file as suggested in PR #4892
  move title out of included file as suggested in PR #4892
  Add possible values for widget_type
  [#4842] Making 2 sentences
  changes as suggested by WouterJ in pull request #4842
  Add meaning of yellow icon for number of queries
  Fixing bad link name
  fix typo in event flow diagrams
  Many fixes thanks to stof, WouterJ, xabbuh and dupuchba
  added Jakub as a merger for the DomCrawler component
  [#5094] Removing mkdir - it's not needed (thanks xabbuh)
  some tweaks to #4601
  Moving index down to correct section
  [#4989] Language tweaks and making the example simpler
  Remove useless setLocale() call and add code block with locale setter
  Finaly touches on translation locale setting note
  Review note about setting the translator locale in a controller.
  Update translation.rst
  ...

Conflicts:
	cookbook/security/entity_provider.rst
weaverryan added a commit that referenced this pull request Mar 24, 2015
@weaverryan
Merge branch '2.6' into 2.7
8566263 
* 2.6: (45 commits)
  Removed duplicate "long"s
  terminate file with newline
  move title back and move description to separate file as suggested in PR #4892
  move title out of included file as suggested in PR #4892
  Add possible values for widget_type
  [#4842] Making 2 sentences
  changes as suggested by WouterJ in pull request #4842
  Add meaning of yellow icon for number of queries
  Fixing bad link name
  fix typo in event flow diagrams
  Many fixes thanks to stof, WouterJ, xabbuh and dupuchba
  added Jakub as a merger for the DomCrawler component
  [#5094] Removing mkdir - it's not needed (thanks xabbuh)
  some tweaks to #4601
  Moving index down to correct section
  [#4989] Language tweaks and making the example simpler
  Remove useless setLocale() call and add code block with locale setter
  Finaly touches on translation locale setting note
  Review note about setting the translator locale in a controller.
  Update translation.rst
  ...
weaverryan added a commit that referenced this pull request Mar 24, 2015
@weaverryan
Merge branch '2.7'
7681e39 
* 2.7: (47 commits)
  Removed duplicate "long"s
  terminate file with newline
  move title back and move description to separate file as suggested in PR #4892
  move title out of included file as suggested in PR #4892
  Add possible values for widget_type
  [#4842] Making 2 sentences
  changes as suggested by WouterJ in pull request #4842
  Add meaning of yellow icon for number of queries
  Fixing bad link name
  fix typo in event flow diagrams
  Many fixes thanks to stof, WouterJ, xabbuh and dupuchba
  added Jakub as a merger for the DomCrawler component
  Changes thanks to WouterJ and xabbuh
  [#5094] Removing mkdir - it's not needed (thanks xabbuh)
  some tweaks to #4601
  Moving index down to correct section
  [#4989] Language tweaks and making the example simpler
  Remove useless setLocale() call and add code block with locale setter
  Finaly touches on translation locale setting note
  Review note about setting the translator locale in a controller.
  ...
@javiereguiluz javiereguiluz deleted the update_cookbook_assetic branch January 3, 2018 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@javiereguiluz @wouterj @weaverryan @xabbuh