-
-
Notifications
You must be signed in to change notification settings - Fork 519
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 nested steps note to step-definitions #89
Conversation
Checks that failed were the wayback urls timing out, nothing that I did. :)
|
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 @jaysonesmith - this is a nice addition! The content is good, but maybe see if you could edit the paragraphs to have shorter, clearer sentences? That would make it even better!
content/step-definitions.md
Outdated
|
||
### Nested Steps | ||
|
||
In an effort to keep your code DRY, you may be tempted to utilize Cucumber's feature of calling step definitions from other steps: **DON'T**. While this may seem like a useful feature to utilize, it creates bad code smells and can make things like readability difficult and stack traces terrible. Instead, in places where you're calling steps from other steps, use those as opportunities to refactor your code to create helper methods to use instead. |
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 explain "DRY" (for non-developers)?
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.
AFAIK nested steps are only possible in Ruby, so mark this text block with {{% text "ruby" %}} {{% /text %}}
Also, java & js examples will not be needed ;)
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.
Something to consider: As this is (also) a Best practice / Anti-pattern, maybe it should be in best-practices rather than step-definitions? On the other hand, this would probably the place where people read about how to write step definitions, so this might actually be the best place.
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.
Also here, see if you can make the sentences shorter & clearer by splitting them or removing words (i.e. duplicate "instead" in last sentence).
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'll rephrase and do a link to the wiki article on DRY. :)
- While not a part of the official Cucumber repo, the implementation of Cucumber in Go also allows for nested steps. Perhaps when trying to inform folks about this, I can just point them to the Ruby version of it instead.
- I'm open to adding a note to the best-practice page too. I'll do a different PR for that once we get this settled.
- Thanks for catching the duplicate instead! Ugh. I'll work on rephrasing, thanks!
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! Currently the polyglot pages have tabs for Java/Javascript/Ruby. Not sure what to do about the other language implementations, as they don't have a place in the nw menu structure anymore...?
content/step-definitions.md
Outdated
``` | ||
{{% /text %}} | ||
|
||
This refactoring has multiple benefits, but core to this topic is that it would then allow you to access this code more easily throughout all of your steps as needed, instead of having to call a step to do it. You get much more flexibility, readabilitiy, and usability with helper methods. Lastly, they won't be deprecated 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.
Make sentences shorter to make them easier to read on a screen; split them into multiple sentences if needed. Here, you might also use a bullit list?
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.
Noted!
@jaysonesmith Also: sometimes HTMLProofer fails on external links, if they don't respond fast enough. If you feel like it, hit "Retry" until they pass.. If you really feel like it, see if you can figure out how to configure it to skip them (#30 - we decided on won't fix earlier, but it's getting kind of annoying...) |
* Remove dead links from blog-posts * Fix date formatting to be consistent
Removed dead links to fix the build and rebased your branch onto master. |
Thanks @mlvandijk ! |
I'm not certain if this is the best place for it, but it makes sense to me! I'm open to feedback and assistance in duplicating my examples in js and java.
p.s. I've got a longer, more article/blog type document written also, but it doesn't fit in the documentation type, style.