Trunk #2153

Merged
merged 8 commits into from May 19, 2014

Conversation

Projects
None yet
4 participants
Owner

fabiopelosin commented May 19, 2014

Closes #2151

@alloy I letting the merge to you in case you have any final requirement.

I think ‘alias’ is the better word in place of ‘placeholder’.

Owner

fabiopelosin replied May 19, 2014

👍

Owner

fabiopelosin replied May 19, 2014

done

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/push.rb
super
end
def validate!
- super
- help! "A spec-repo name is required." unless @repo
+ UI.puts '[!] The `pod push` command has been moved to `pod repo push`.'.ansi.yellow
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [91/80]

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+
+ # @!group Push sub-steps
+
+ extend Executable
+ executable :git
+
+ # Performs a full lint against the podspecs.
+ #
+ def validate_podspec_files
+ UI.puts "\nValidating #{'spec'.pluralize(count)}".yellow
+ podspec_files.each do |podspec|
+ validator = Validator.new(podspec)
+ validator.only_errors = @allow_warnings
+ begin
+ validator.validate
+ rescue Exception
@houndci-bot

houndci-bot May 19, 2014

Avoid rescuing the Exception class.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ # @return [Array<Pathname>] The path of the specifications to push.
+ #
+ def podspec_files
+ files = Pathname.glob(@podspec || "*.podspec")
+ raise Informative, "Couldn't find any .podspec file in current directory" if files.empty?
+ files
+ end
+
+ # @return [Integer] The number of the podspec files to push.
+ #
+ def count
+ podspec_files.count
+ end
+
+ #---------------------------------------------------------------------#
+
@houndci-bot

houndci-bot May 19, 2014

Extra empty line detected at body end.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
@@ -0,0 +1,178 @@
+require 'fileutils'
+require 'active_support/core_ext/string/inflections'
+
+module Pod
+ class Command
+ class Repo < Command
+ class Push < Repo
+ self.summary = 'Push new specifications to a spec-repo'
+
+ self.description = <<-DESC
+ Validates NAME.podspec or `*.podspec' in the current working dir, creates
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [81/80]

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
@@ -0,0 +1,178 @@
+require 'fileutils'
+require 'active_support/core_ext/string/inflections'
+
+module Pod
+ class Command
+ class Repo < Command
+ class Push < Repo
+ self.summary = 'Push new specifications to a spec-repo'
+
+ self.description = <<-DESC
+ Validates NAME.podspec or `*.podspec' in the current working dir, creates
+ a directory and version folder for the pod in the local copy of
+ REPO (~/.cocoapods/repos/[REPO]), copies the podspec file into the version
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [82/80]

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+
+ self.description = <<-DESC
+ Validates NAME.podspec or `*.podspec' in the current working dir, creates
+ a directory and version folder for the pod in the local copy of
+ REPO (~/.cocoapods/repos/[REPO]), copies the podspec file into the version
+ directory, and finally it pushes REPO to its remote.
+ DESC
+
+ self.arguments = [
+ ['REPO', :required],
+ ['NAME.podspec', :optional]
+ ]
+
+ def self.options
+ [ ["--allow-warnings", "Allows pushing even if there are warnings"],
+ ["--local-only", "Does not perform the step of pushing REPO to its remote"] ].concat(super)
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [103/80]
Space inside square brackets detected.
Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ # @!group Push sub-steps
+
+ extend Executable
+ executable :git
+
+ # Performs a full lint against the podspecs.
+ #
+ def validate_podspec_files
+ UI.puts "\nValidating #{'spec'.pluralize(count)}".yellow
+ podspec_files.each do |podspec|
+ validator = Validator.new(podspec)
+ validator.only_errors = @allow_warnings
+ begin
+ validator.validate
+ rescue Exception
+ raise Informative, "The `#{podspec}` specification does not validate."
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [84/80]

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ extend Executable
+ executable :git
+
+ # Performs a full lint against the podspecs.
+ #
+ def validate_podspec_files
+ UI.puts "\nValidating #{'spec'.pluralize(count)}".yellow
+ podspec_files.each do |podspec|
+ validator = Validator.new(podspec)
+ validator.only_errors = @allow_warnings
+ begin
+ validator.validate
+ rescue Exception
+ raise Informative, "The `#{podspec}` specification does not validate."
+ end
+ raise Informative, "The `#{podspec}` specification does not validate." unless validator.validated?
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [110/80]
Use fail instead of raise to signal exceptions.

@AliSoftware

AliSoftware May 19, 2014

Contributor

side note, shouldn't we fix the hound config as we are using raise everywhere and not fail?

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ clean = Dir.chdir(repo_dir) { `git status --porcelain 2>&1` } == ''
+ raise Informative, "The repo `#{@repo}` is not clean" unless clean
+ end
+
+ # Updates the git repo against the remote.
+ #
+ # @return [void]
+ #
+ def update_repo
+ UI.puts "Updating the `#{@repo}' repo\n".yellow
+ Dir.chdir(repo_dir) { UI.puts `git pull 2>&1` }
+ end
+
+ # Commits the podspecs to the source, which should be a git repo.
+ #
+ # @note The pre commit hook of the repo is skipped as the podspecs have
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [81/80]

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ # @return [void]
+ #
+ def update_repo
+ UI.puts "Updating the `#{@repo}' repo\n".yellow
+ Dir.chdir(repo_dir) { UI.puts `git pull 2>&1` }
+ end
+
+ # Commits the podspecs to the source, which should be a git repo.
+ #
+ # @note The pre commit hook of the repo is skipped as the podspecs have
+ # already been linted.
+ #
+ # @return [void]
+ #
+ def add_specs_to_repo
+ UI.puts "\nAdding the #{'spec'.pluralize(count)} to the `#{@repo}' repo\n".yellow
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [91/80]

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+
+ # @!group Private helpers
+
+ # @return [Pathname] The directory of the repository.
+ #
+ def repo_dir
+ dir = config.repos_dir + @repo
+ raise Informative, "`#{@repo}` repo not found" unless dir.exist?
+ dir
+ end
+
+ # @return [Array<Pathname>] The path of the specifications to push.
+ #
+ def podspec_files
+ files = Pathname.glob(@podspec || "*.podspec")
+ raise Informative, "Couldn't find any .podspec file in current directory" if files.empty?
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [99/80]
Use fail instead of raise to signal exceptions.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ end
+
+ # Checks that the repo is clean.
+ #
+ # @raise If the repo is not clean.
+ #
+ # @todo Add specs for staged and unstaged files.
+ #
+ # @todo Gracefully handle the case where source is not under git
+ # source control.
+ #
+ # @return [void]
+ #
+ def check_repo_status
+ clean = Dir.chdir(repo_dir) { `git status --porcelain 2>&1` } == ''
+ raise Informative, "The repo `#{@repo}` is not clean" unless clean
@houndci-bot

houndci-bot May 19, 2014

Use fail instead of raise to signal exceptions.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ def push_repo
+ UI.puts "\nPushing the `#{@repo}' repo\n".yellow
+ Dir.chdir(repo_dir) { UI.puts `git push origin master 2>&1` }
+ end
+
+ #---------------------------------------------------------------------#
+
+ private
+
+ # @!group Private helpers
+
+ # @return [Pathname] The directory of the repository.
+ #
+ def repo_dir
+ dir = config.repos_dir + @repo
+ raise Informative, "`#{@repo}` repo not found" unless dir.exist?
@houndci-bot

houndci-bot May 19, 2014

Use fail instead of raise to signal exceptions.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ self.summary = 'Push new specifications to a spec-repo'
+
+ self.description = <<-DESC
+ Validates NAME.podspec or `*.podspec' in the current working dir, creates
+ a directory and version folder for the pod in the local copy of
+ REPO (~/.cocoapods/repos/[REPO]), copies the podspec file into the version
+ directory, and finally it pushes REPO to its remote.
+ DESC
+
+ self.arguments = [
+ ['REPO', :required],
+ ['NAME.podspec', :optional]
+ ]
+
+ def self.options
+ [ ["--allow-warnings", "Allows pushing even if there are warnings"],
@houndci-bot

houndci-bot May 19, 2014

Space inside square brackets detected.
Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ def self.options
+ [ ["--allow-warnings", "Allows pushing even if there are warnings"],
+ ["--local-only", "Does not perform the step of pushing REPO to its remote"] ].concat(super)
+ end
+
+ def initialize(argv)
+ @allow_warnings = argv.flag?('allow-warnings')
+ @local_only = argv.flag?('local-only')
+ @repo = argv.shift_argument
+ @podspec = argv.shift_argument
+ super
+ end
+
+ def validate!
+ super
+ help! "A spec-repo name is required." unless @repo
@houndci-bot

houndci-bot May 19, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ ["--local-only", "Does not perform the step of pushing REPO to its remote"] ].concat(super)
+ end
+
+ def initialize(argv)
+ @allow_warnings = argv.flag?('allow-warnings')
+ @local_only = argv.flag?('local-only')
+ @repo = argv.shift_argument
+ @podspec = argv.shift_argument
+ super
+ end
+
+ def validate!
+ super
+ help! "A spec-repo name is required." unless @repo
+ if @repo == 'master'
+ help! "To push to the master repo use the `pod trunk push` command"
@houndci-bot

houndci-bot May 19, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ podspec_files.each do |spec_file|
+ spec = Pod::Specification.from_file(spec_file)
+ output_path = File.join(repo_dir, spec.name, spec.version.to_s)
+ if Pathname.new(output_path).exist?
+ message = "[Fix] #{spec}"
+ elsif Pathname.new(File.join(repo_dir, spec.name)).exist?
+ message = "[Update] #{spec}"
+ else
+ message = "[Add] #{spec}"
+ end
+
+ FileUtils.mkdir_p(output_path)
+ FileUtils.cp(spec_file, output_path)
+ Dir.chdir(repo_dir) do
+ # only commit if modified
+ if git!("status --porcelain 2>&1").include?(spec.name)
@houndci-bot

houndci-bot May 19, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on the diff May 19, 2014

lib/cocoapods/command/repo/push.rb
+ private
+
+ # @!group Private helpers
+
+ # @return [Pathname] The directory of the repository.
+ #
+ def repo_dir
+ dir = config.repos_dir + @repo
+ raise Informative, "`#{@repo}` repo not found" unless dir.exist?
+ dir
+ end
+
+ # @return [Array<Pathname>] The path of the specifications to push.
+ #
+ def podspec_files
+ files = Pathname.glob(@podspec || "*.podspec")
@houndci-bot

houndci-bot May 19, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on the diff May 19, 2014

spec/functional/command/repo/push_spec.rb
@@ -20,7 +20,7 @@ module Pod
it "complains if it can't find a spec" do
repo_make('test_repo')
- e = lambda { run_command('push', 'test_repo') }.should.raise Pod::Informative
+ e = lambda { run_command('repo', 'push', 'test_repo') }.should.raise Pod::Informative
@houndci-bot

houndci-bot May 19, 2014

Line is too long. [91/80]

Owner

alloy commented May 19, 2014

From skimming, it looks good. Did you try to perform a push with both the real and the aliased commands?

Owner

fabiopelosin commented May 19, 2014

I didn't perform a push, I got the commands to the failure point of the missing podspec (to ensure that they actually run). There should be no issues... but, you're right better to do a last minute check before the release.

Contributor

AliSoftware commented May 19, 2014

Yeah, "just a little quick fix without testing it right before the big release", right right… 😆

fix in production

Owner

fabiopelosin commented May 19, 2014

Tested works as expected!

@fabiopelosin fabiopelosin added a commit that referenced this pull request May 19, 2014

@fabiopelosin fabiopelosin Merge pull request #2153 from CocoaPods/trunk
Trunk
13f5a37

@fabiopelosin fabiopelosin merged commit 13f5a37 into master May 19, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

fabiopelosin deleted the trunk branch May 19, 2014

This needs to check against the git remote URL, other people are using the name ‘master’.

Owner

alloy commented May 19, 2014

Tested works as expected!

Superb work, good sir!

Owner

fabiopelosin commented May 19, 2014

This needs to check against the git remote URL, other people are using the name ‘master’.

Can you clarify?

Contributor

AliSoftware commented May 19, 2014

@irrationalfab That's what I was talking about in the TODO issue

Contributor

AliSoftware commented May 19, 2014

(there)

Owner

fabiopelosin commented May 19, 2014

@AliSoftware got that I was looking for a rationale from Eloy because we always promoted the convention that master is our repo, so I don't think that many people are using the name for private repos or are using a different name for the master repo. Moreover the comparison is used only for a friendly notice and if it fails nothing happens as people cannot push to master anymore.

Owner

alloy commented May 20, 2014

Can you clarify?

@irrationalfab During the test period we had running, I noticed that many people push private specs to their fork of master, but leave it with that name. We do NOT want them to accidentally push private specs.

Owner

fabiopelosin commented May 20, 2014

@alloy so you fear that they will use trunk after seeing the warning?

Owner

alloy commented May 20, 2014

@irrationalfab Yeah, I’m never surprised by the ability of people to do the wrong thing ;) but mainly because the code you have right now will not let them push at all in case of their repo being named ‘master’ and being told to use pod trunk push instead.

Owner

fabiopelosin commented May 20, 2014

As there is not much room for the discussion I will use your approach, still I think that educating users that the master repo by convention is intended to be the CocoaPods master repo could prevent some troubles in future.

Contributor

AliSoftware commented May 20, 2014

still I think that educating users that […]

It's nice to see that there are still dreamers in there that think that all users are able to be educated 😆

Owner

alloy commented May 20, 2014

[Update] Oops, replaying to old comment.

Owner

alloy commented May 20, 2014

So yes, they should be educated indeed, but with the right info. E.g.:

  • You are using the master repo to add your own private specs, don’t do this.
  • You are using the name ‘master’ for you own private spec-repo, don’t do this.
Contributor

AliSoftware commented May 20, 2014

@alloy 👍

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