Skip to content

Use proper logger in deploy strategy #222

Merged
merged 1 commit into from Jul 26, 2012

3 participants

@MasterLambaster

There is no :logger configuration variable, logger is accessed via instance variable.
Because of that all deploy strategies creates new logger to STDOUT.

@leehambley
Capistrano member

@MasterLambaster any chance you can test for this? I wouldn't like to break it when working in this area.

@MasterLambaster

@leehambley I've updated existing tests. On my local copy with custom logger

self.logger = Capistrano::Logger.new(:output => (File.expand_path('../../log/deploy.log', __FILE__)))
self.logger.level = 3

I have following output to STDOUT

* getting (via checkout) revision 703296f20df74867a7a3bc725f800380de41969a to /var/folders/9w/_89sl6ds6nj9wvf46zgdv_tc0000gn/T/20120606131240
  executing locally: git clone
* Compressing /var/folders/9w/_89sl6ds6nj9wvf46zgdv_tc0000gn/T/20120606131240 to /var/folders/9w/_89sl6ds6nj9wvf46zgdv_tc0000gn/T/20120606131240.tar.gz
    executing locally: tar czf 20120606131240.tar.gz 20120606131240
    command finished in 50ms

With that patch everything goes to log as expected. I assume that we do not need separate log for deploy strategies and its descendants. Moreover we are testing strategies in isolation without using actual Capistrano::Configuration` so i have no idea how that wrong behavior can be covered in strategy tests. All test in Capistrano stubs config.logger not config[:logger].

@leehambley
Capistrano member

All clear Alex, thanks for the additional information.

@carsomyr

@MasterLambaster Would you be so kind as to collapse your two commits?

@carsomyr carsomyr merged commit ffe860f into capistrano:master Jul 26, 2012
@carsomyr

Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.