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

Update assetic watch command #4657

Closed
wants to merge 3 commits into from
Closed

Update assetic watch command #4657

wants to merge 3 commits into from

Conversation

geerteltink
Copy link

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

I'm getting this message:

The --watch option is deprecated. Please use the assetic:watch command instead.

I'm getting this message:

    The --watch option is deprecated. Please use the assetic:watch command instead.
@javiereguiluz
Copy link
Member

@xtreamwayz thanks for updating this command!

When Symfony changes a well-known feature, option or command, we usually add a message in the newer version of the docs to help experienced Symfony developers realize that things have changed. In this case, we could add something like the following:

.. versionadded:: 2.6
    Prior to Symfony 2.6 the ``assetic:watch`` command was executed via the
    ``--watch`` option of the ``assecit:dump`` command.

In any case, let's see what do our code reviewers (@wouterj and @xabbuh) think about this idea.

@xabbuh
Copy link
Member

xabbuh commented Dec 16, 2014

@xtreamwayz Good catch and very valuable change.

As @javiereguiluz said, you simply have to add the versionadded directive. According to the doc standards, it should be worded like:

.. versionadded:: 2.6
    The ``assetic:watch`` command was introduced in Symfony 2.6. In prior
    versions, you had to use the ``--watch`` option of the ``assetic:dump``
    command for the same behavior.

@stof
Copy link
Member

stof commented Dec 16, 2014

This is not related to Symfony 2.6. It is related to AsseticBundle 2.4, which is compatible with Symfony 2.1+

@geerteltink
Copy link
Author

Updated the text. Not sure if this is better. If not, feel free to adjust.

@wouterj
Copy link
Member

wouterj commented Feb 21, 2015

I like it (and sorry for missing this PR). I've tagged it with finished, so the merger will see that this PR is mergable.

Note for @weaverryan, this should be merged into the 2.3 branch.

@xabbuh
Copy link
Member

xabbuh commented Feb 21, 2015

@xtreamwayz Sorry for not coming back to you after you applied the changes. I like the way you worded the sentence. 👍

@weaverryan
Copy link
Member

Nice job guys, with the versionadded on the AsseticBundle version, I think it's the only way. I am merging this into 2.3, though there is some risk - the VAST majority of 2.3 projects will likely use the older AsseticBundle version. But, I think it'll be a really thing to be able to find and fix. Anyways, thanks @xtreamwayz!

@weaverryan weaverryan closed this Feb 24, 2015
weaverryan added a commit that referenced this pull request Feb 24, 2015
This PR was submitted for the 2.6 branch but it was merged into the 2.3 branch instead (closes #4657).

Discussion
----------

Update assetic watch command

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

I'm getting this message:

    The --watch option is deprecated. Please use the assetic:watch command instead.

Commits
-------

1009ed4 Typo
987a31a Improve assetic:watch text
8c38ec8 Update asset_management.rst
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.

6 participants