Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Improved pod list #188

Merged
merged 12 commits into from Mar 26, 2012

Conversation

Projects
None yet
2 participants
Owner

fabiopelosin commented Mar 24, 2012

Inspired by Specs#79. The code is still a bit a ugly.

IMG

$ pod list --help
List all pods:

    $ pod list

      Lists all available pods.

    $ pod list [DAYS]

      Lists the pods introduced in the master repo since the given number of days.

Options
-------

    --stats     Show additional stats (like GitHub watchers and forks)
    --help      Show help information
    --silent    Print nothing
    --verbose   Print more information while working
    --version   Prints the version of CocoaPods

@alloy alloy and 1 other commented on an outdated diff Mar 25, 2012

lib/cocoapods/command/print_pod.rb
+ class Command
+ module DisplayPods
+
+ def display_pod_list(array, stats = false)
+ array.each do |set|
+ puts_pod(set, stats)
+ end
+ end
+
+ def puts_pod(set, stats = false)
+ puts "\e[32m--> #{set.name} (#{set.versions.reverse.join(", ")})\e[0m"
+ puts_wrapped_text(set.specification.summary)
+
+ spec = set.specification.part_of_other_pod? ? set.specification.part_of_specification : set.specification
+
+ source = spec.source.reject {|k,_| k == :commit || k == :tag }.values.first
@alloy

alloy Mar 25, 2012

Owner

Oops, good catch :)

@fabiopelosin

fabiopelosin Mar 26, 2012

Owner

Honestly, I introduced the bug... the previous code was good.

@alloy alloy and 1 other commented on an outdated diff Mar 25, 2012

lib/cocoapods/command.rb
@@ -1,6 +1,7 @@
module Pod
class Command
autoload :ErrorReport, 'cocoapods/command/error_report'
+ autoload :DisplayPods, 'cocoapods/command/print_pod'
@alloy

alloy Mar 25, 2012

Owner

Can you name the file after the module? display_pods.rb

@fabiopelosin

fabiopelosin Mar 26, 2012

Owner

Oops, I forgot.

Owner

alloy commented Mar 25, 2012

I like it, but I’d like to see some unit tests for the individual methods. That code is becoming more complex and could probably be refactored a bit afterwards.

FYI Another thing I often do in cases like this is sort the list of names.

Owner

fabiopelosin replied Mar 26, 2012

Good to now, thanks for the info.

This was failing on Travis and the failure seems related to the following line. As I never used Travis, does somebody have an idea about why was it failing?

repo_info = `curl -s -m 2 http://github.com/api/v2/json/repos/show/#{username}/#{reponame}`
Owner

alloy replied Mar 26, 2012

Since Travis runs on linux, there are often differences in order and sometimes with command-line options. I would start by copy-pasting the line from the Travis log that actually runs bacon and the files. Sometimes you can reproduce a bug simply by running the same command again.

If that doesn’t fix it, then you’d have to check on Travis. I have no problem with you printing out all sorts of info from the specs so that you can diagnose the problem :) Worst case scenario, you’d have to run the Travis VM locally, I already have it, so in that case I can take a loofa for you.

Owner

fabiopelosin replied Mar 26, 2012

Mhmmm... now it works, but I didn't change anything. A network failure? (It happend only in ruby 1.9.3).

Owner

alloy replied Mar 26, 2012

Oh yeah, that’s also very possible. Sometimes the Travis stack has an issue and the tests won’t even run, but it will still report that as a failing build… :-/

Owner

fabiopelosin replied Mar 26, 2012

Mhmmm... I think that something more is going on. The failure reappeared again in the same line. It might be because curl is called with timeout of 2s (I didn't want to block search for too long). That should be an eternity for such a small request. @alloy what do your suggest? To remove the timeout?

Owner

alloy replied Mar 26, 2012

It might be better to use something else than curl all together. E.g.:

require 'net/http'

stats = Net::HTTP.get('example.com', '/index.html')

This can then be stubbed in the tests like so:

Net::HTTP.expects(:get).with('example.com', '/index.html').returns("the data")
command.run
Owner

fabiopelosin commented Mar 26, 2012

@alloy What do you think? Is it ready?

Owner

alloy commented Mar 26, 2012

Yup, looks good, go gO GO! :)

@fabiopelosin fabiopelosin merged commit 22e342a into master Mar 26, 2012

jzapater pushed a commit to jzapater/CocoaPods that referenced this pull request Sep 17, 2013

fabiopelosin added a commit that referenced this pull request Oct 25, 2014

fabiopelosin added a commit that referenced this pull request Oct 25, 2014

fabiopelosin added a commit that referenced this pull request Oct 25, 2014

fabiopelosin added a commit that referenced this pull request Oct 25, 2014

fabiopelosin added a commit that referenced this pull request Oct 25, 2014

fabiopelosin added a commit that referenced this pull request Oct 25, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment