Trigger kde release #24
Conversation
There was a problem hiding this comment.
I've already reviewed this and didn't like the way it was duplicating code
#23 (review)
| l.level = Logger::INFO | ||
| end | ||
|
|
||
| if release.nil? or not KDEProjectsComponent.respond_to?(release) |
There was a problem hiding this comment.
Use || instead of or.
Use ! instead of not.
What you write there is the overly complicated variant of unless release && KDEProjectsComponent.respond_to?(release)
| end.parse! | ||
|
|
||
| @log = Logger.new(STDOUT).tap do |l| | ||
| l.progname = 'retry' |
There was a problem hiding this comment.
This isn't called retry is it?
| opts.banner = <<-HELPTEXT | ||
| Triggers the watcher jobs for when a new release of Frameworks/Plasma/Applications is made. | ||
|
|
||
| Usage: jenkins_trigger_new_release_build.rb -r [plasma,applications,frameworks] |
There was a problem hiding this comment.
#{$0} instead of jenkins_trigger_new_release_build.rb
| class << self | ||
|
|
There was a problem hiding this comment.
trailing spaces
&
excess newline
| @@ -18,13 +18,15 @@ | |||
| # You should have received a copy of the GNU Lesser General Public | |||
| # License along with this library. If not, see <http://www.gnu.org/licenses/>. | |||
|
|
|||
| # downloads and makes available as arrays lists of KDE projects which | |||
| # downloads and makes available as arrays lists of jobs which | |||
| def to_names(projects) | ||
| projects.collect { |project| project.split('/')[-1] } | ||
| def to_names(project_list) | ||
| project_list.collect! { |project| project.split('/')[-1] } |
There was a problem hiding this comment.
This edits the caller's list. You probably should avoid this by not using collect! and assign a new variable instead. Editing a caller's object will almost certainly cause confusion further down the line.
| # are part of Plasma, Applications and Frameworks | ||
|
|
||
| require 'httparty' | ||
|
|
||
| class KDEProjectsComponent | ||
| @projects_to_jobs = {'kirigami'=> 'kirigami2', 'discover'=> 'plasma-discover'} |
There was a problem hiding this comment.
Space inside { missing.
Surrounding space missing for operator =>.
Space inside } missing.
This should probably be a constant, not a class instance variable.
| projects.collect { |project| project.split('/')[-1] } | ||
| def to_names(project_list) | ||
| project_list.collect! { |project| project.split('/')[-1] } | ||
| @projects_to_jobs.each do |project_name, job_name| |
There was a problem hiding this comment.
This entire block is better expressed as
project_list.collect { |x| @projects_to_jobs.fetch(x, x) }
which re-collects the array, by running each value through the hash with both the value as key AND default value. i.e. fetch('foobar', 'foobar') = if 'foobar' is in the hash it will return the value it is mapped to, otherwise it will return 'foobar'
| @log.info 'Setting system into maintenance mode.' | ||
| Jenkins.system.quiet_down | ||
|
|
||
| BlockingThreadPool.run do |
hsitter
force-pushed
the
master
branch
2 times, most recently
from
4d67ffe to
114a632
Compare
hsitter
force-pushed
the
master
branch
6 times, most recently
from
2d5fb19 to
014e8b9
Compare
No description provided.