-
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
Add "Writing a custom Behat formatter" cookbook #149
base: v3.0
Are you sure you want to change the base?
Conversation
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.
@jdeniau thanks very much for this, it's a great addition to the documentation.
I've made some suggestions for style / grammar things that it would be good to fix - I feel bad picking them out, as there's no way I could write anything like this in French!
If you're OK with my suggestions I'd be happy to apply them for you.
@acoulton do not feel bad, English is not my main language and I make a lot of errors. I make typos too 🙂 I'm OK with all your suggestions. I'm not on a computer right now, so if you want to make the changes yourself no problem for me. If not, I will commit this in the next days. |
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.
Thanks @jdeniau I've added those edits for you.
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.
It's a really good addition. The only changes I'd request would be to replace "behat" with "Behat" and try and stay away from terms like "simple" when trying to explain a step. It's not a "must solve" on my end, but something that's simple to a lot of people might not be simple for others.
Thanks @ttomdewit I missed some of the I'll take another pass later unless @jdeniau gets there first. |
8223594
to
fb03842
Compare
@ttomdewit I pulled it into my IDE to find and replace all the lowercase |
cookbooks/custom_formatter.rst
Outdated
|
||
default: | ||
extensions: | ||
JDeniau\BehatReviewdogFormatter\ReviewdogFormatterExtension: ~ |
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.
if you put your extension class in a ServiceContainer
subnamespace, with a name matching the last section of your namespace, this unlocks using the shortcut.
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.
Hi,
I do not understand what you mean here: is there a way to "magically" add an extension without being explicit in the configuration file ?
I read https://github.com/Behat/Behat/blob/610fa32937bf52bd79d812103464600ef4649e36/src/Behat/Testwork/ServiceContainer/ContainerLoader.php and https://github.com/Behat/Behat/blob/610fa32937bf52bd79d812103464600ef4649e36/src/Behat/Testwork/ServiceContainer/ExtensionManager.php source code, but I did not see anything about it 🤔
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.
You got very close - the magic is here (and the method it calls)
Basically if you define your class named MyNamespace\MyPackage\ServiceContainer\MyPackageExtension
then in the config file you can just use MyNamespace\MyPackage
and Behat will find the class.
However I actually think now that we support php config files the short syntax is less useful (people can just add use statements like anywhere else) and we could probably deprecate it at some point.
So I wouldn't worry about it for this PR.
Oh I did though that it had been merged. I will try to change that 👍 |
a1db0e6
to
2deede1
Compare
@carlos-granados I did change according to @stof recommandations except for #149 (comment) that I did not understand. The CI is failing due to lines too long, but those lines does contains links or are in bullets points, and I don't know how to split those lines. |
Thanks for this @jdeniau I will take a look properly soon. There have been some improvements to Behat since you wrote it, I may be able to suggest a couple of changes to include them. |
Fixes #143