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
Open
4 changes: 3 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ PATH
remote: .
specs:
github-team-remover (0.0.2)
colorize
octokit (~> 3.0)
thor

Expand All @@ -10,12 +11,13 @@ GEM
specs:
addressable (2.3.6)
ast (1.1.0)
colorize (0.7.2)
diff-lcs (1.2.5)
faraday (0.9.0)
multipart-post (>= 1.2, < 3)
json (1.8.1)
multipart-post (2.0.0)
octokit (3.0.0)
octokit (3.1.0)
sawyer (~> 0.5.3)
parser (2.1.7)
ast (~> 1.1)
Expand Down
1 change: 1 addition & 0 deletions github-team-remover.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Gem::Specification.new do |spec|

spec.add_dependency 'octokit', '~> 3.0'
spec.add_dependency 'thor'
spec.add_dependency 'colorize'

spec.add_development_dependency 'rubocop'
spec.add_development_dependency 'rspec', '~> 3.0.0.beta2'
Expand Down
45 changes: 38 additions & 7 deletions lib/remover/cli.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,56 @@
require 'thor'
require 'colorize'

module Remover
class CLI < Thor
method_option :organization, required: true
method_option :login, required: true
method_option :password, required: true

desc('list', 'List unused teams')
method_option :organization, :aliases => '--o', required: true
method_option :login, :aliases => '--l', required: true
method_option :password, :aliases => '--p', required: true
method_option :color, :aliases => '--c'
method_option :verbose, :aliases => '--v'

desc('|Commands:', 'Alias, Command, Meaning.')

def list
Remover.configuration.load_from_options!(options)

Remover::List.new(github).unused_teams.each do |team|
puts team.name
puts 'Unused teams:'.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

def github
Remover::Github.new(octokit)
end
Expand Down
18 changes: 18 additions & 0 deletions lib/remover/commander.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
require 'thor'
require 'colorize'
require 'octokit'

module Remover
class Commander < Thor

def color
end

def verbose
end

def delete
end

end
end
2 changes: 1 addition & 1 deletion lib/remover/configuration.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module Remover
class Configuration
OPTIONS = %i(organization login password)
OPTIONS = %i(organization login password color verbose)

attr_accessor(*OPTIONS)

Expand Down
34 changes: 34 additions & 0 deletions lib/remover/team.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,40 @@ 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?

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

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

private

def with_members?
Expand Down
3 changes: 3 additions & 0 deletions spec/remover/commander_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
describe Remover::Commander do

end
6 changes: 5 additions & 1 deletion spec/remover/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
{
'organization' => 'FlatSchool',
'login' => 'fs-school',
'password' => '123456'
'password' => '123456',
'color' => 'blue',
'verbose' => 'true'
}
end

Expand All @@ -18,6 +20,8 @@
expect(configuration.organization).to eq('FlatSchool')
expect(configuration.login).to eq('fs-school')
expect(configuration.password).to eq('123456')
expect(configuration.color).to eq('blue')
expect(configuration.verbose).to eq('true')
end
end
end
46 changes: 45 additions & 1 deletion 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') }
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) }

Expand All @@ -9,6 +9,50 @@
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('Github Member')] }

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

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
end
end

describe '#repositories_amount' do
let(:repositories_amount) { [double(1)] }

before do
allow(github_client).to receive(:team_repositories) { repositories_amount }
end

context 'must be equal' do
it 'returns amount of repos' do
expect(team.repositories_amount.size).to eq(github_client.repositories_amount.size)
end
end
end

describe '#used' do
let(:members) { [double('Github Member')] }
let(:repositories) { [double('Github Repository')] }
Expand Down