Copy stratery refactoring #185

Merged
merged 26 commits into from Apr 12, 2012

Conversation

Projects
None yet
3 participants
@despo
Contributor

despo commented Mar 18, 2012

refactoring copy strategy to make more readable

@leehambley

This comment has been minimized.

Show comment
Hide comment
@leehambley

leehambley Mar 19, 2012

Member

@despo what provision is made so that people relying on monkey patching the existing copy strategy can continue to use their code?

The list has spoken a few times about refactoring this, but decided that actually, we need to maintain a stable API.

Member

leehambley commented Mar 19, 2012

@despo what provision is made so that people relying on monkey patching the existing copy strategy can continue to use their code?

The list has spoken a few times about refactoring this, but decided that actually, we need to maintain a stable API.

@despo

This comment has been minimized.

Show comment
Hide comment
@despo

despo Mar 19, 2012

Contributor

@leehambley I wasn't aware of the discussion in the list and I refactored in an attempt to understand what goes on in the copy strategy.

I have renamed run_build_script back to build so that it doesn't impact anyone who has overridden the method.
As I have only extracted the deploy method to smaller private methods, I'm not sure how this would impact any monkey patching. Then again, I can't be sure...

Keeping hold of code just for the sake of not breaking existing build scripts doesn't make a lot of sense. Can't the version be bumped and a warning or update be issued instead? The code is a lot more readable now and if anyone decides to update, they should easily be able to apply any changes. (Plus anyone wanting to use the existing copy strategy should now be able to extend it easier without having to patch deploy/build or copy cache)

Contributor

despo commented Mar 19, 2012

@leehambley I wasn't aware of the discussion in the list and I refactored in an attempt to understand what goes on in the copy strategy.

I have renamed run_build_script back to build so that it doesn't impact anyone who has overridden the method.
As I have only extracted the deploy method to smaller private methods, I'm not sure how this would impact any monkey patching. Then again, I can't be sure...

Keeping hold of code just for the sake of not breaking existing build scripts doesn't make a lot of sense. Can't the version be bumped and a warning or update be issued instead? The code is a lot more readable now and if anyone decides to update, they should easily be able to apply any changes. (Plus anyone wanting to use the existing copy strategy should now be able to extend it easier without having to patch deploy/build or copy cache)

@dfreudenberger

This comment has been minimized.

Show comment
Hide comment
@dfreudenberger

dfreudenberger Mar 28, 2012

+1 on this - just tried to read through the existing code which isn't really easy

+1 on this - just tried to read through the existing code which isn't really easy

@despo

This comment has been minimized.

Show comment
Hide comment
@despo

despo Mar 29, 2012

Contributor

Any suggestions on how to improve this code, so the request can make it in, are more than welcome.

Contributor

despo commented Mar 29, 2012

Any suggestions on how to improve this code, so the request can make it in, are more than welcome.

@leehambley

This comment has been minimized.

Show comment
Hide comment
@leehambley

leehambley Mar 29, 2012

Member

I'll pull it the next time I find an hour to work on Cap. (2+ people was
enough to convince me, and I actually don't care a enough about the copy
strategy to argue with people who are depending on it)

Member

leehambley commented Mar 29, 2012

I'll pull it the next time I find an hour to work on Cap. (2+ people was
enough to convince me, and I actually don't care a enough about the copy
strategy to argue with people who are depending on it)

leehambley added a commit that referenced this pull request Apr 12, 2012

Merge pull request #185 from despo/copy_stratery_refactoring
Copy stratery refactoring. (This might break your monkey patching)

@leehambley leehambley merged commit c8139bf into capistrano:master Apr 12, 2012

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