Skip to content
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

Jump to bottom

added new module: cucumber-needle #500

Merged
merged 10 commits into from Aug 14, 2013
Merged

added new module: cucumber-needle #500

merged 10 commits into from Aug 14, 2013

Conversation

jangalinski
Copy link
Contributor

@see Issue #496

aslakhellesoy
aslakhellesoy reviewed Apr 7, 2013
View reviewed changes
java/src/main/java/cucumber/runtime/java/ObjectFactory.java
@@ -1,11 +1,30 @@
package cucumber.runtime.java;

public interface ObjectFactory {

/**
* Setup factory <b>before</b> feature execution. Called once per feature.
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment is incorrect. It's called once for every scenario.

aslakhellesoy
aslakhellesoy reviewed Apr 8, 2013
View reviewed changes
needle/src/test/java/cucumber/runtime/java/test/injectionprovider/ValueInjectionProvider.java
/**
* Returns a value provider returning the value given in constructor.
*
* @author Simon Zambrovski
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this author tag

@aslakhellesoy
Copy link
Contributor

There is a lot of logging output printed to STDOUT when running the build. Can you please silence it or send it to a file instead?

@jangalinski
Copy link
Contributor Author

Logging is silenced now. Thanks for your patience. Good night :-)

aslakhellesoy
aslakhellesoy reviewed Apr 10, 2013
View reviewed changes
needle/src/main/java/de/akquinet/jbosscc/needle/injection/DefaultInstanceInjectionProvider.java
@@ -0,0 +1,61 @@
package de.akquinet.jbosscc.needle.injection;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't accept classes that don't live in the cucumber.* package

@aslakhellesoy
Copy link
Contributor

Can you please rebase or merge this PR with master? It no longer applies cleanly.

@jangalinski
Copy link
Contributor Author

Will take care of your comments tonight.

@jangalinski
Copy link
Contributor Author

Finally had a moment to integrate your suggestions.

@jangalinski
Copy link
Contributor Author

Whats the status of this request? Do you have everything you need to merge?

@aslakhellesoy
Copy link
Contributor

I've been to busy lately, sorry. I'll take another look when I have more
time.

Aslak

On Thursday, May 9, 2013, Jan Galinski wrote:

Whats the status of this request? Do you have everything you need to merge?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/500#issuecomment-17658562
.

aslakhellesoy added a commit that referenced this pull request Aug 14, 2013
@aslakhellesoy
Minor tweaks to needle module. Closes #496, #500
09848fa
@aslakhellesoy aslakhellesoy merged commit c6f3111 into cucumber:master Aug 14, 2013
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jangalinski @aslakhellesoy