1
\$\begingroup\$
#!/usr/bin/env ruby

require 'pivotal-tracker'
require 'yaml'

TEMP_DIR = "#{ENV['HOME']}/.pivotal/" 
TEMP_FILE = TEMP_DIR + "temp.yml"
CONFIG_FILE = TEMP_DIR + "config.yml"

unless File.directory? TEMP_DIR
  Dir::mkdir TEMP_DIR
end

unless File.file? TEMP_FILE
  f = File.open(TEMP_FILE, 'w')
  f.write("id: -1")
end

unless File.file? CONFIG_FILE
  f = File.open(CONFIG_FILE, 'w')
  f.write("")
end
# TEMP FILE format:
  # current_story_id: 123 OR -1

def story_info story
  puts story.name
  puts "id:\t\t#{story.id}"
  puts "notes:\t\t#{story.description}"
  puts "status:\t\t#{story.current_state}"
  puts "estimate:\t#{(story.estimate == -1) ? 'unestimated' : story.estimate}"
end

def is_next string
  string == 'next'
end

def id string
  !string.match(/^[\d]+(\.[\d]+){0,1}$/).nil?
end

def current string
  string == 'current'
end

def story_has_been_started
  f = YAML.load_file(TEMP_FILE)
  @current_id = f['id']
  f['id'] != -1
end

def update_id id
  f = File.open(TEMP_FILE, 'w')
  f.write("id: #{id}")
  @current_id = id
end

def set_config key, value
  config = YAML::load(File.read(CONFIG_FILE)) || {}
  config[key.to_sym] = value
  open(CONFIG_FILE, 'w') { |f| YAML::dump(config, f) }
end

def authenticate
  config = YAML::load(File.read(CONFIG_FILE)) || {}
  has_email = true, has_password = true

  unless config.has_key? :email
    puts "\033[33mUse pivotal set email EMAIL to set your pivotal tracker email\033[0m\n"
    has_email = false
  end

  unless config.has_key? :password
    puts "\033[33mUse pivotal set password PASSWORD to set your pivotal tracker email\033[0m\n"
    has_password = false
  end

  unless has_email and has_password
    exit 0
  end
  PivotalTracker::Client.token(config[:email], config[:password])
end

@current_id = nil

begin
  unless ARGV[0] == 'set'
    authenticate
    aidin = PivotalTracker::Project.all.first
  end
rescue
  puts "\033[33mInvalid Credentials\033[0m\n"
  exit 0
end

case ARGV[0]
when "info"
  # pivotal info next
  # pivotal info id
  # pivotal info current

  if is_next(ARGV[1])
    puts "\033[32mDisplaying information for next story\033[0m\n"
    story_info(aidin.stories.all(owner: 'Chintan Parikh', state: 'unstarted').first)

  elsif id(ARGV[1])
    story = aidin.stories.find(Integer(ARGV[1]))
    unless story.nil?
      puts "\033[32mDisplaying information for story #{ARGV[1]}\033[0m\n"
      story_info(story)
    else
      puts "\033[33mNo story with id #{ARGV[1]} exists\033[0m\n"
    end

  elsif current(ARGV[1])
    if story_has_been_started
      story = aidin.stories.find(@current_id)
      puts "\033[32mDisplaying information for current story\033[0m\n"
      story_info(story)
    else    
      puts "\033[33mNo story has been started. Use pivotal start to start a story first\033[0m\n"
    end
  end

when "estimate"
  # pivotal estimate next 1-8
  # pivotal estimate id 1-8
  # pivotal estimate current 1-8

when "start"
  # pivotal start next
  # pivotal start id
  if story_has_been_started
    current_branch = `git branch | grep "*" | sed "s/* //"`.chomp
    status = `git status -s`
    if !status.empty?
      puts "\033[33mYou are currently working on story #{@current_id} and have uncommitted changes on #{current_branch}. If you continue, your uncommitted changes will be lost. Continue? (Y/N)\033[0m\n"
      continue = false
      while (!continue)
        option = $stdin.gets.chomp
        if option == 'N'
          exit 1
        elsif option != 'Y'
          puts "Please enter either Y or N"
        else
          continue = true;
        end
      end
    end
  end

  `git stash`
  `git stash drop`
  `git checkout develop`
  `git pull`

  if is_next(ARGV[1])
    story = aidin.stories.all(owner: 'Chintan Parikh', state: 'unstarted').first
    new_branch = "feature/#{story.id}_#{story.name.downcase.gsub(' ', '_').gsub(/[^0-9A-Za-z_]/, '')}"
    `git checkout -b #{new_branch}`
  elsif id(ARGV[1])
    story = aidin.stories.find(Integer(ARGV[1]))
    unless story.nil?
      new_branch = "feature/#{story.id}_#{story.name.downcase.gsub(' ', '_').gsub(/[^0-9A-Za-z_]/, '')}"
      `git checkout -b #{new_branch}`
    else
      puts "\033[33mNo story with id #{ARGV[1]} exists\033[0m\n"
    end
  end

  update_id(story.id) unless story.nil?
  story.update(current_state: 'started')

  puts "\033[32mStory #{story.id} has been started\033[0m\n"
  story_info(story)

when "complete"
  if story_has_been_started
    story = aidin.stories.find(@current_id)
    branch = "feature/#{story.id}_#{story.name.downcase.gsub(' ', '_').gsub(/[^0-9A-Za-z_]/, '')}"
    `git push origin #{branch}`
    story.update(current_state: 'finished')
    story.update(current_state: 'delivered')

    puts "\033[32mStory #{story.id} has been completed\033[0m\n"
    update_id(-1)
  else
    puts "\033[33mNo story has been started. Use pivotal start to start a story first\033[0m\n"
  end

when 'abandon'
  if story_has_been_started
    story = aidin.stories.find(@current_id)
    story.update(current_state: 'unstarted')

    puts "\033[32mStory #{story.id} has been abandoned\033[0m\n"
    update_id(-1)
  else
    puts "\033[33mNo story has been started. Use pivotal start to start a story first\033[0m\n"
  end

when 'list'
  stories = aidin.stories.all(owner: 'Chintan Parikh', current_state: ['unstarted', 'started', 'finished', 'delivered'])
  stories.each do |story|
    story_info(story)
  end
when 'set'
  # set email
  # set password
  set_config(ARGV[1], ARGV[2])
  puts "\033[32m#{ARGV[1]} has been set to #{ARGV[2]}\033[0m\n"
end

It's a pretty basic application to simplify my workflow where I'm interning. I'd like to chuck it on my github, but I'd rather the code was a little nicer before I throw it up for the world to see. Any idea where, or how I should start refactoring?

\$\endgroup\$
  • 2
    \$\begingroup\$ wants code to be "nicer before I throw it up for the world to see" - posts code on the internet anyway :) But in all seriousness, I'd probably start by removing the hard coded name. I'm thinking it should be possible to get your name via the pivotal library you're using, since you already give it an email and password (and that config might work better as environment variables or a ~/.pivotalconfig - or something - file) \$\endgroup\$ – Flambino May 29 '13 at 21:40
  • \$\begingroup\$ You should really try rubocop, it is fascinating! \$\endgroup\$ – Mhmd May 17 '15 at 10:57
4
\$\begingroup\$

I haven't used Pivotal Tracker, so I can't really say what's "right" to do, purpose-wise. However, skimming through your code, I have a few ideas:

1. Extend PivotalTracker's classes for fun and profit

Instead of a global method like story_info (which I'd prefer was called print_story_info as that what it does), I'd probably extend the PivotalTracker::Story class with an info, inspect, or to_s method, so you can simply write puts story.info when necessary. Something like

module PivotalTracker
  class Story
    def info
      <<-EOT
      #{name}
      ID:       #{id}
      Notes:    #{description}
      Status:   #{story.current_state}
      Estimate: #{estimate == -1 ? 'none' : estimate}
      EOT
    end
  end
end

Do the same to add a class method for finding "your" stories, and whatever else you need to do with Pivotal.

2. Those string-checking functions...

Your is_next, id and current methods smell pretty grim. For one, it should - if anything - be next?, id?, and current?, but really there's no need for them to begin with. You only use them twice in your code. Yes, yes, DRY and all that, but honestly, a simple string == "next" comparison does not need to be abstracted into a method.

Now, if you were doing something more fault-tolerant like downcasing before comparing, or allowing short-hand args like "n" for "next", then maybe. But even so, the methods would still smell. Remember, in Ruby you can use regular expressions directly in string switches, e.g.

case some_string
when /^n(ext)?$/i
  # do stuff
case...

That said, your code could absolutely benefit from encapsulating its logic in more methods and classes, but this isn't the place to start.

I'd recommend using an option/command line switch parser to do the ARGV-checking/-translation heavy-lifting, and in general restructuring stuff. Right now, your script is basically bash written in Ruby, with big, global switches and all. But it could be a lot cleaner with more OOP and a few helpful gems.

3. Hand-coded terminal styling

Hand-coding the styles makes the code harder to read and more error-prone (cosmetic errors, but still). Use a coloring gem of some sort to do that stuff for you (and to check if the terminal even supports colors and styles!)

4. Check that shell commands actually succeed

In your start, er, "function", you perform a lot of git commands, some very destructive. I'd do some serious checking before and after that. In particular I'd at least check $?.success? a few times to make sure that a command actually succeeded before running the next one.

5. The estimate argument

All commented out → just remove it.
Just saying :)

Also see the comment I posted to your question (i.e. don't hardcode your name in there)

\$\endgroup\$
  • \$\begingroup\$ Brilliant, thanks so much! I'll get to work :) \$\endgroup\$ – user1045696 May 30 '13 at 1:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.