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

Max Larionov - Github-team-remover progress(Finished.) #3

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
added --verbose, will make another class for reporting soon
  • Loading branch information
maxinspace committed Apr 24, 2014
commit 29e78b2445931086edf848535911e192614d1ba7
31 changes: 25 additions & 6 deletions lib/remover/cli.rb
Original file line number Diff line number Diff line change
@@ -15,19 +15,38 @@ class CLI < Thor
def list
Remover.configuration.load_from_options!(options)
puts 'Unused teams:'.colorize(color)

Remover::List.new(github).unused_teams.each do |unused_team|
puts "
Team name: #{unused_team.name},
Members: #{unused_team.members_amount},
Repositories: #{unused_team.repositories_amount}".colorize(color)
if verbose?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create separata class for outputting invormation.
Logic should be incapsulated there.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will do it today and global refactoring.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got really huge troubles with making another class, code not working on my PC, but works on another.

Remover::List.new(github).unused_teams.each do |unused_team|
puts "
Team name: #{unused_team.name},
Members: #{unused_team.members_amount},
Members URL: #{unused_team.members_url},
Repositories: #{unused_team.repositories_amount},
Repositories URL: #{unused_team.repositories_url}
".colorize(color)
end
else
Remover::List.new(github).unused_teams.each do |unused_team|
puts "
Team name: #{unused_team.name},
Members: #{unused_team.members_amount},
Repositories: #{unused_team.repositories_amount}".colorize(color)
end
end
end

default_task :list

private

def verbose?
if ! options[:verbose].eql? nil
true
else
false
end
end

def color
Remover.configuration.color.to_sym
end
31 changes: 28 additions & 3 deletions lib/remover/team.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# added members & repos
module Remover
class Team
attr_accessor :github_client, :github_team
@@ -15,12 +14,38 @@ def name
github_team.name
end

def members_url
if with_members?
hash = Hash(*github_client.team_members(github_team.id))
hash[:html_url]
else
'no members'
end
end

def repositories_url
if with_repositories?
hash = Hash(*github_client.team_repositories(github_team.id))
hash[:html_url]
else
'no repositories'
end
end

def members_amount
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please use these new methods in the with_members? and with_repositories? also?

github_client.team_members(github_team.id).size
if with_members?
github_client.team_members(github_team.id).size
else
0
end
end

def repositories_amount
github_client.team_repositories(github_team.id).size
if with_repositories?
github_client.team_repositories(github_team.id).size
else
0
end
end

private
22 changes: 19 additions & 3 deletions spec/remover/team_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
describe Remover::Team do
let(:github_client) { double('Github Client', members_amount: 1, repositories_amount: 1) }
let(:github_client) { double('Github Client', members_amount: 1, repositories_amount: 1, members_url: 'url') }
let(:github_team) { double('Github Team', id: 1, name: 'Owners') }
let(:team) { Remover::Team.new(github_client, github_team) }

@@ -9,14 +9,30 @@
end
end


describe '#members_url' do
let(:members_url) {[double('Github Member')]}

before do
allow(github_client).to receive(:team_members) {members_url}
end

context 'members_url' do
it 'returns members url' do
expect(team.members_url).to eq(github_client.members_url)
end
end
end


describe '#members' do
let(:members_amount) { [double(1)] }
let(:members_amount) { [double('Github Member')] }

before do
allow(github_client).to receive(:team_members) { members_amount }
end

context 'must be equal' do
context 'memebers amount' do
it 'returns amount of members' do
expect(team.members_amount.size).to eq(github_client.members_amount.size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you count size of integer here?
As far as I understand team.members_amount returns interger alread.

end