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.

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

Cleanup of Features management #47

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

jorgeejgonzalez
Copy link
Contributor

changed cucumber.runtime.arquillian.feature.Features class name to FeaturesManager;
broke createFeatureMap method into smaller private methods and rearranged the content of the class;
changed the accesors of the other public methods as they are not called anywhere outside of the FeaturesManager (nor do they seem like they should be)

*: I don't really get the whole findWithCucumberSearcher name, as far as I see it you're loading a resource from where you load the URLs of the features, so I kept the CucumberSearcher part in the extractUrlMap and createList method but I don't really get it.

**: I ran all the tests and everything seems fine this time

@@ -1,278 +1,278 @@
package cucumber.runtime.arquillian.client;
Copy link
Member

Choose a reason for hiding this comment

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

this diff shouldnt be on the whole file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, not sure why it shows it that way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be the distance of the changes: line 11, line 64, line 225 and line 276

@jorgeejgonzalez
Copy link
Contributor Author

I returned the test to its previous state and reduced the number of private methods by one level. I also added a helper class to handle the urlPath, urlSuffix, featureHome, etc. that kept being passed between methods, this way I reduced the list of params in some functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants