-
Notifications
You must be signed in to change notification settings - Fork 70
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
chore: update docs to use Attributes instead of Annotations #178
chore: update docs to use Attributes instead of Annotations #178
Conversation
I don't think the Rector 2.0 release is a blocker for updating our documentation (it is a blocker for deprecating annotations as we want to make Rector the migration tool) |
Yeah, if we agree this is ready before Rector is ready, we can release these changes without the mention to Rector, let's see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a couple of small suggestions. Thanks!
quick_start.rst
Outdated
Behat uses PHP Attributes for step definitions, step | ||
transformations and hooks. It can also use doc-block annotations | ||
and this will be described in the User Guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe more like this, with a link to the new page?
Behat uses PHP Attributes for step definitions, step
transformations and hooks. It also supports doc-block
annotations for compatibility with legacy code, but this
syntax is deprecated - seethe annotations documentation
for details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
quick_start.rst
Outdated
looks like ``... there is a ..., which costs £...``. This pattern will match | ||
any of the following steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should make it even more explicit that #[Given] / #[When] / #[Then]
are completely interchangeable? This section does already say the pattern will match any of the following steps
but I think with attributes it's even more surprising that we have 3 attributes that do the same thing...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think you might suggest a text for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like ``... there is a ..., which costs £...``. This pattern will match | |
any of the following steps: | |
looks like ``... there is a ..., which costs £...``. | |
The ``#[Given]``, ``#[When]`` and ``#[Then]`` attributes are functionally identical - they | |
only exist separately to help keep the wording of your step definitions readable. So | |
this pattern will match any of the following steps: |
quick_start.rst
Outdated
Also, Behat will not add the ``use`` statements for the step attributes for you, you | ||
may need to add them manually if they were not yet present in your context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How hard would it be to make it that it does? If that's too hard, should we generate attributes with FQCN and it's up to the user to shorten them if they want?
I can see it tripping a lot of people up if their file looks right but it's not matching anything because they've not noticed they need to import the attributes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added an issue to our Behat repo to handle this. Until this issue is done we can keep this comment here. The issue is: Behat/Behat#1553
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is now going to be solved in Behat/Behat#1549 so we can probably take it out?
user_guide/annotations.rst
Outdated
transformations in PHP contexts. These annotations are still available and you can use them instead | ||
of PHP attributes in your projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be more forceful here.
transformations in PHP contexts. These annotations are still available and you can use them instead | |
of PHP attributes in your projects. | |
transformations in PHP contexts. These annotations are still available for now, and you can use them instead | |
of PHP attributes in your projects - however they will likely be deprecated and removed in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
user_guide/annotations.rst
Outdated
* @When event occurs | ||
*/ | ||
public function doSomeAction() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth showing with a parameter, so people can more easily see it's the same as an attribute?
* @When event occurs | |
*/ | |
public function doSomeAction() | |
* @When an :event occurs | |
*/ | |
public function onEvent(string $event) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
user_guide/annotations.rst
Outdated
------------- | ||
|
||
Even though annotations are still available, they will probably be deprecated and eventually removed in the future. | ||
In this way, for new projects we recommend that you don't use them. If your current project uses annotations, we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this way, for new projects we recommend that you don't use them. If your current project uses annotations, we | |
Therefore, we do not recommend using annotations for new projects. If your current project uses annotations, we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
user_guide/annotations.rst
Outdated
recommend that you refactor your code to use PHP attributes instead. This can be done manually but you should be able | ||
to use the search and replace capabilities of your IDE to do this in a more automated way. | ||
|
||
Alternatively you may want to use a tool like `Rector`_ which can do automated refactoring of your code. Rector includes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could just say Rector 2.0 includes a rule...
even if this is ready before the release is out. That's enough for people to work out whether to try it with an RC / wait for it IMO and saves waiting or coming back to this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, suggested wording for the interchangeable attributes as requested.
quick_start.rst
Outdated
Also, Behat will not add the ``use`` statements for the step attributes for you, you | ||
may need to add them manually if they were not yet present in your context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is now going to be solved in Behat/Behat#1549 so we can probably take it out?
quick_start.rst
Outdated
looks like ``... there is a ..., which costs £...``. This pattern will match | ||
any of the following steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like ``... there is a ..., which costs £...``. This pattern will match | |
any of the following steps: | |
looks like ``... there is a ..., which costs £...``. | |
The ``#[Given]``, ``#[When]`` and ``#[Then]`` attributes are functionally identical - they | |
only exist separately to help keep the wording of your step definitions readable. So | |
this pattern will match any of the following steps: |
1445acf
to
738b879
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great :)
This should not be merged yet, I would like to wait for two things:
Closes #177