Pluralize 'spec(s) per process' #792
Conversation
|
@grosser Ah, I had thought it was in Ruby, but maybe I’m wrong about that! How about something simple like the following? "#{name}#{'s' if tests_per_process > 1} per process"or maybe "#{name}#{'s' unless tests_per_process == 1} per process" |
|
that works ... can put it into a |
brchristian
force-pushed
the
pluralize
branch
from
564dae8
to
26d8662
Compare
|
Good point! Although I would worry about namespacing conflicts with the Rails one. Because it only gets used in this one place, maybe it’s best to just do it inline. If we find ourselves using it in other places we can always DRY it up later. That’s my thinking! But LMK if you disagree and I am happy to create a method. Otherwise I have force-pushed and I think we’re looking good. |
|
Actually, you’ve convinced me! |
brchristian
force-pushed
the
pluralize
branch
from
26d8662
to
ef0ad39
Compare
lib/parallel_tests/cli.rb
Outdated
| def pluralize(n, singular, plural=nil) | ||
| if n == 1 | ||
| "1 #{singular}" | ||
| elsif plural | ||
| "#{n} #{plural}" |
There was a problem hiding this comment.
we never pass plural 🤷
| def pluralize(n, singular, plural=nil) | |
| if n == 1 | |
| "1 #{singular}" | |
| elsif plural | |
| "#{n} #{plural}" | |
| def pluralize(n, singular) | |
| if n == 1 | |
| "1 #{singular}" |
There was a problem hiding this comment.
The idea was to include future proofing for weird stuff like person/people. :) If it’s cleaner to remove it, no problem!
There was a problem hiding this comment.
Okay, pushed with your changes!
brchristian
force-pushed
the
pluralize
branch
from
ef0ad39
to
59c5769
Compare
lib/parallel_tests/cli.rb
Outdated
| @@ -142,7 +142,17 @@ def report_number_of_tests(groups) | |||
| num_processes = groups.size | |||
| num_tests = groups.map(&:size).inject(0, :+) | |||
| tests_per_process = (num_processes == 0 ? 0 : num_tests / num_processes) | |||
| puts "#{num_processes} processes for #{num_tests} #{name}s, ~ #{tests_per_process} #{name}s per process" | |||
| puts "#{pluralize(num_processes, 'process'} for #{pluralize(num_tests, name)}, ~ #{pluralize(tests_per_process, name)} per process" | |||
There was a problem hiding this comment.
| puts "#{pluralize(num_processes, 'process'} for #{pluralize(num_tests, name)}, ~ #{pluralize(tests_per_process, name)} per process" | |
| puts "#{pluralize(num_processes, 'process')} for #{pluralize(num_tests, name)}, ~ #{pluralize(tests_per_process, name)} per process" |
There was a problem hiding this comment.
Whoops, you’re 100% correct! Sorry about that. Force-pushed!
brchristian
force-pushed
the
pluralize
branch
from
59c5769
to
b6cf361
Compare
|
tests need some fixes since they assert |
brchristian
force-pushed
the
pluralize
branch
from
b6cf361
to
c2e2664
Compare
|
Tests are now updated with regex! |
|
thx, will be in next release whenever that comes around :) |
Thank you for your contribution!
Checklist
master(if not - rebase it).code introduces user-observable changes.