Skip to content

[Stopwatch] Describe retrieval of StopwatchEvent #3539

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

Merged
merged 4 commits into from
Mar 26, 2014
Merged

[Stopwatch] Describe retrieval of StopwatchEvent #3539

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2ca9bd5
Describe retrieval of StopwatchEvent
jochenvdv Feb 4, 2014
ac484f4
Made suggested tweaks
jochenvdv Mar 25, 2014
295f109
Add reason for getEvent()
jochenvdv Mar 25, 2014
0ee15d3
Fix versionadded
jochenvdv Mar 25, 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
2 changes: 2 additions & 0 deletions components/stopwatch.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ microtime by yourself. Instead, use the simple
// ... some code goes here
$event = $stopwatch->stop('eventName');

The :class:`Symfony\\Component\\Stopwatch\StopwatchEvent` object can be retrieved from :method:`Symfony\\Component\\Stopwatch\\Stopwatch::start`, :method:`Symfony\\Component\\Stopwatch\\Stopwatch::stop`, :method:`Symfony\\Component\\Stopwatch\\Stopwatch::lap` and :method:`Symfony\\Component\\Stopwatch\\Stopwatch::getEvent`
Copy link
Contributor

Choose a reason for hiding this comment

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

i believe you could add the reason why there is a need to obtain the event class which you did say it on your other PR

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think that's a good suggestion


Copy link
Member

Choose a reason for hiding this comment

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

When we use :method:, only the function name will show up. So, this will read a bit better like this I think:

The :class:`Symfony\\Component\\Stopwatch\StopwatchEvent` object can be retrieved from 
the :method:`Symfony\\Component\\Stopwatch\\Stopwatch::start`, 
:method:`Symfony\\Component\\Stopwatch\\Stopwatch::stop`,
:method:`Symfony\\Component\\Stopwatch\\Stopwatch::lap` and
:method:`Symfony\\Component\\Stopwatch\\Stopwatch::getEvent` methods.

Also, can you add line breaks so that we have a line break after the first word that crosses the 72nd character? With these long :method: things, we often will put those entirely on their own line.

We usually also add a versionadded:: 2.5 tag above a feature that is new, but I'm not sure here - it seems like a lot of work just for the 1 new method getEvent. Let's see what others think.

Thanks!

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 the versionadded directive must be present here. Otherwise, after 2.5 has been released, you'll automatically get this version of the docs by default which could then be confusing if you are using an older Symfony version and don't notice that you can't use the getEvent() method then.

Thus, I'd add this above the new paragraph:

.. versionadded:: 2.5
    The ``getEvent()`` method was introduced in Symfony 2.5.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, let's add the versionadded

You can also provide a category name to an event::

$stopwatch->start('eventName', 'categoryName');
Expand Down