Skip to content
This repository was archived by the owner on Dec 21, 2024. It is now read-only.

Conversation

@ralphkar
Copy link
Contributor

Cucumber 5 brings new capabilities for using an object factory. I extended the document about state with additional information on how to use Cucumber in a DI environment.

@mpkorstanje
Copy link
Contributor

Cheers! I'll try and pick it up soonish. Unless some one else gets to it before me.

@ralphkar
Copy link
Contributor Author

The PR is now hanging here for almost 2 weeks. Should I just merge it?

@mpkorstanje
Copy link
Contributor

I've had this on my list to review for a while. Please be patient.

Copy link
Member

@mlvandijk mlvandijk left a comment

Choose a reason for hiding this comment

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

Thank you for this addition! We really appreciate all the effort. As I'm not familiar with Guice, I can't comment on the content for Guice. (I'd have to try this out, but am not able to do so right now).

I do have some general request regarding writing style, to make it more consistent with the other docs, and easier to read.
Mainly:

  1. please don't use terms like "simple","simply". I understand this is probably intended to put the reader at ease ("its's not hard, you can do it!") but for someone who does not yet understand how to do something, this could be demotivating / condescending.
  2. Could you make the text more concise please? This includes linking to other sections for more details, where relevant (to prevent duplication of content in multiple places)
    I've given some suggestions on both; please have a look and accept the ones you agree with.

Finally, if this functionality is available as of v5, let's merge after v5 has been released.
Thanks for your patience and help!

@ralphkar
Copy link
Contributor Author

Enhanced readability and conciseness

@mlvandijk
Copy link
Member

Much more concise, thanks!

@mlvandijk mlvandijk requested a review from mpkorstanje October 29, 2019 08:31
@mpkorstanje mpkorstanje merged commit 7abb352 into cucumber:master Oct 31, 2019
@mpkorstanje
Copy link
Contributor

Took me a while to get around to this but it looks good to me! Thanks a bucket.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants