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

[Refactoring] Splitting responsabilities between classes and helpers #11

Closed

Conversation

bogdanRada
Copy link
Collaborator

@bogdanRada bogdanRada commented Jun 28, 2016

I am working on a bit of refactoring . I honestly didn't liked the fact that i have put all the methods for Capistrano 2 and 3 in a single helper.
So i am trying to split a bit the responsabilities in multiple helpers.

I opened this pull request in order to get some feedback. This would help us maintain this gem more easily in the future. I have tested a bit the changes with applications that use capistrano version 3 and 2 and seems to work ok.

Right now i am more interested in getting some feedback. Maybe there are other ways we could improve this. I am opened to suggestions.

@bogdanRada
Copy link
Collaborator Author

bogdanRada commented Jun 28, 2016

Or another solution would be to use classes . The main concern i have is that we are actually polluting right now the Kernel and Object with our methods from the current helper .
And i would like to not do that anymore. But the only way i see that working is if we use classes instead of helpers and send the capistrano instance to the class or something similar.

I kept for now the way we have right now with helpers but i could try that approach with classes if you think would be better.

@bogdanRada
Copy link
Collaborator Author

I am working on a different approach. I think i will close this for now and open a new one when it will be ready for review.

@bogdanRada bogdanRada closed this Jul 6, 2016
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.

1 participant