Skip to content
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

provide node path on configuration #4249

Merged
merged 4 commits into from
Sep 18, 2014
Merged

provide node path on configuration #4249

merged 4 commits into from
Sep 18, 2014

Conversation

desarrolla2
Copy link
Contributor

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

If you no provide node path in assetic configuration, UglifyCssFilter throw a RuntimeException: Path to node executable could not be resolved.

If you no provide node path in assetic configuration, UglifyCssFilter throw a  `RuntimeException: Path to node executable could not be resolved.`
@xabbuh
Copy link
Member

xabbuh commented Sep 18, 2014

Actually, as long as the name of the binary is node and it can be located by ExecutableFinder or if it can be found under /usr/bin/node, it's not necessary to configure anything here. Maybe you should add a comment mentioning this.

@desarrolla2
Copy link
Contributor Author

@xabbuh i was updated the PR as you indicate. In my ubuntu14.04 i was installed node via apt and binary name is nodejs

@@ -111,6 +111,34 @@ your JavaScripts:

You now have access to the ``uglifyjs2`` filter in your application.

Configure the ``node binary``
----------------------------------
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 use the name number of - characters as there are characters in the headline.

Copy link
Member

Choose a reason for hiding this comment

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

The format that we use for the documentation is very strict for things like the length of the headings underlining. So please, reduce the underneath line length to match the length of the title. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

The funny thing is that reST only complain about underlines that are too short.

Copy link
Member

Choose a reason for hiding this comment

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

@xabbuh reST allows longer underlines. Matching the length exactly is a matter of coding standards

@desarrolla2
Copy link
Contributor Author

I think that i solved your comments

@javiereguiluz
Copy link
Member

@desarrolla2 thanks for your quick response to the comments and for having created this documentation.

I know that you've been a Symfony developer for a long time, but if I'm right, this is your first documentation contribution. If this is true, congratulations!

By the way, you mentioned the How to Contribute to Symfony Documentation guide that we recently updated. If you found something hard to understand or some missing explanation, please feel free to open an issue in this repository. Thanks.

-----------------------------

Assetic tries to find the node binary automatically. If it cannot be found, you'll
be able to configure its location using the ``node`` key:
Copy link
Member

Choose a reason for hiding this comment

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

How about:

If it cannot be found, you can configure its location...

@weaverryan
Copy link
Member

I just left a comment, but actually, this looks great to me. So I'm going to merge and change the language myself in that tiny spot. Thanks so much for this!

@weaverryan weaverryan merged commit ab3dbef into symfony:2.3 Sep 18, 2014
weaverryan added a commit that referenced this pull request Sep 18, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

provide node path on configuration

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

If you no provide node path in assetic configuration, UglifyCssFilter throw a  `RuntimeException: Path to node executable could not be resolved.`

Commits
-------

ab3dbef update uglifyjs.rst
1a5c0a6 Update uglifyjs.rst
1621071 Update uglifyjs.rst
2258ac9 provide node path on configuration
weaverryan added a commit that referenced this pull request Sep 18, 2014
weaverryan added a commit that referenced this pull request Sep 18, 2014
* 2.3:
  [#4249] Tiny tweak to language
  update uglifyjs.rst
  Update uglifyjs.rst
  Update uglifyjs.rst
  provide node path on configuration
weaverryan added a commit that referenced this pull request Sep 18, 2014
* 2.4:
  [#4249] Tiny tweak to language
  update uglifyjs.rst
  Update uglifyjs.rst
  Update uglifyjs.rst
  provide node path on configuration
weaverryan added a commit that referenced this pull request Sep 18, 2014
* 2.5:
  [#4249] Tiny tweak to language
  update uglifyjs.rst
  Update uglifyjs.rst
  Update uglifyjs.rst
  provide node path on configuration
daFish pushed a commit to daFish/symfony-docs that referenced this pull request Sep 28, 2014
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.

5 participants