Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Display external requirements for gem versions #502

Closed
wants to merge 1 commit into from

4 participants

Ian Morgan Amos King Evan Phoenix Postmodern
Ian Morgan

Displays external gem requirements in the same area as dependencies. Should close #489

Amos King adkron commented on the diff
app/views/rubygems/show.html.erb
@@ -100,6 +100,18 @@
<%= render :partial => "rubygems/dependencies", :locals => { :dependencies => @latest_version.dependencies.runtime } %>
<%= render :partial => "rubygems/dependencies", :locals => { :dependencies => @latest_version.dependencies.development } %>
+
+ <% if @latest_version.requirements.present? %>
+ <div class="dependencies requirements_dependencies">
+ <h5><%= t '.requirements_header' %></h5>
+ <ol>
+ <% Array(@latest_version.requirements).each do |requirement| %>
Amos King Collaborator
adkron added a note

Why is there a cast to an Array here? requirements is already an array, correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Amos King adkron commented on the diff
app/models/version.rb
@@ -8,6 +8,7 @@ class Version < ActiveRecord::Base
after_save :reorder_versions
serialize :licenses
+ serialize :requirements
Amos King Collaborator
adkron added a note

I think that it would be a good move to make a Requirement object. Then in the future it would be easier to add queries based on that. ie. Find all gems with a specific requirement, find gems with no external dependencies. This would help users if they want to find pure ruby gems for instance. It would also make for a nice place to hang behavior later.

Serialized objects don't work well for queries and writing a migration later would be harder than doing this now.

gemspec.requirements is not especially common, even less so than licenses which is a serialized column in this manner. I had thought a full model was overkill. If that's the way you'd like this done i'll scrap this and go that route.

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

I'm a committer, but I'm not running the show. I don't think that anyone would complain if it was another collection.

Serializations are not the best when it comes to queries. It makes the design more flexible, and opens up a much larger world for the future.

I would remove the cast to an Array in the view. That is unless I'm missing something.

I would personally be in favor of this if it was not a serialized field.

Evan Phoenix
Owner

Merged! Thanks!

Evan Phoenix evanphx closed this
Nick Quaranto qrush referenced this pull request
Closed

Display gemspec.requirements #489

Postmodern

zomg finally! Now gem authors can expose external dependencies, directly on rubygems.org.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 20, 2012
  1. Ian Morgan
This page is out of date. Refresh to see the latest.
5 app/models/version.rb
View
@@ -8,6 +8,7 @@ class Version < ActiveRecord::Base
after_save :reorder_versions
serialize :licenses
+ serialize :requirements
Amos King Collaborator
adkron added a note

I think that it would be a good move to make a Requirement object. Then in the future it would be easier to add queries based on that. ie. Find all gems with a specific requirement, find gems with no external dependencies. This would help users if they want to find pure ruby gems for instance. It would also make for a nice place to hang behavior later.

Serialized objects don't work well for queries and writing a migration later would be harder than doing this now.

gemspec.requirements is not especially common, even less so than licenses which is a serialized column in this manner. I had thought a full model was overkill. If that's the way you'd like this done i'll scrap this and go that route.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
validates :number, :format => {:with => /\A#{Gem::Version::VERSION_PATTERN}\z/}
validates :platform, :format => {:with => Rubygem::NAME_PATTERN}
@@ -144,6 +145,7 @@ def update_attributes_from_gem_specification!(spec)
:description => spec.description,
:summary => spec.summary,
:licenses => spec.licenses,
+ :requirements => spec.requirements,
:built_at => spec.date,
:indexed => true
)
@@ -186,7 +188,8 @@ def payload
'summary' => summary,
'platform' => platform,
'prerelease' => prerelease,
- 'licenses' => licenses
+ 'licenses' => licenses,
+ 'requirements' => requirements
}
end
12 app/views/rubygems/show.html.erb
View
@@ -100,6 +100,18 @@
<%= render :partial => "rubygems/dependencies", :locals => { :dependencies => @latest_version.dependencies.runtime } %>
<%= render :partial => "rubygems/dependencies", :locals => { :dependencies => @latest_version.dependencies.development } %>
+
+ <% if @latest_version.requirements.present? %>
+ <div class="dependencies requirements_dependencies">
+ <h5><%= t '.requirements_header' %></h5>
+ <ol>
+ <% Array(@latest_version.requirements).each do |requirement| %>
Amos King Collaborator
adkron added a note

Why is there a cast to an Array here? requirements is already an array, correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ <li><%= requirement %></li>
+ <% end %>
+ </ol>
+ </div>
+ <% end %>
+
</div>
</div>
<% end %>
1  config/locales/en.yml
View
@@ -160,6 +160,7 @@ en:
show_all_versions: "Show all versions (%{count} total)"
licenses_header: Licenses
no_licenses: N/A
+ requirements_header: Requirements
searches:
show:
9 db/migrate/20121220014214_add_requirements_to_versions.rb
View
@@ -0,0 +1,9 @@
+class AddRequirementsToVersions < ActiveRecord::Migration
+ def self.up
+ add_column :versions, :requirements, :string
+ end
+
+ def self.down
+ add_column :versions, :requirements, :string
+ end
+end
3  db/schema.rb
View
@@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20121124000000) do
+ActiveRecord::Schema.define(:version => 20121220014214) do
create_table "announcements", :force => true do |t|
t.text "body"
@@ -145,6 +145,7 @@
t.boolean "latest"
t.string "full_name"
t.string "licenses"
+ t.string "requirements"
end
add_index "versions", ["built_at"], :name => "index_versions_on_built_at"
4 features/push.feature
View
@@ -6,8 +6,8 @@ Feature: Push Gems
Scenario: User pushes new gem and sees metadata
Given I am signed up as "email@person.com"
And I have a gem "RGem" with version "1.2.3" and the following attributes:
- | authors | description | license |
- | John Doe | The best gem | MIT |
+ | authors | description | license | requirements |
+ | John Doe | The best gem | MIT | Opencv |
And I have an API key for "email@person.com/password"
When I push the gem "RGem-1.2.3.gem" with my API key
And I visit the gem page for "RGem"
1  test/factories.rb
View
@@ -81,6 +81,7 @@
number
platform "ruby"
licenses "MIT"
+ requirements "Opencv"
rubygem
end
1  test/test_helper.rb
View
@@ -78,6 +78,7 @@ def gem_spec(opts = {})
s.description = %q{This is my awesome gem.}
s.email = %q{joe@user.com}
s.licenses = %w(MIT BSD)
+ s.requirements = %w(Opencv)
s.files = [
"README.textile",
"Rakefile",
6 test/unit/version_test.rb
View
@@ -11,7 +11,7 @@ class VersionTest < ActiveSupport::TestCase
should "only have relevant API fields" do
json = @version.as_json
- assert_equal %w[number built_at summary description authors platform prerelease downloads_count licenses].map(&:to_s).sort, json.keys.sort
+ assert_equal %w[number built_at summary description authors platform prerelease downloads_count licenses requirements].map(&:to_s).sort, json.keys.sort
assert_equal @version.authors, json["authors"]
assert_equal @version.built_at, json["built_at"]
assert_equal @version.description, json["description"]
@@ -21,6 +21,7 @@ class VersionTest < ActiveSupport::TestCase
assert_equal @version.prerelease, json["prerelease"]
assert_equal @version.summary, json["summary"]
assert_equal @version.licenses, json["licenses"]
+ assert_equal @version.requirements, json["requirements"]
end
end
@@ -31,7 +32,7 @@ class VersionTest < ActiveSupport::TestCase
should "only have relevant API fields" do
xml = Nokogiri.parse(@version.to_xml)
- assert_equal %w[number built-at summary description authors platform prerelease downloads-count licenses].map(&:to_s).sort, xml.root.children.map{|a| a.name}.reject{|t| t == "text"}.sort
+ assert_equal %w[number built-at summary description authors platform prerelease downloads-count licenses requirements].map(&:to_s).sort, xml.root.children.map{|a| a.name}.reject{|t| t == "text"}.sort
assert_equal @version.authors, xml.at_css("authors").content
assert_equal @version.built_at.to_i, xml.at_css("built-at").content.to_time.to_i
assert_equal @version.description, xml.at_css("description").content
@@ -41,6 +42,7 @@ class VersionTest < ActiveSupport::TestCase
assert_equal @version.prerelease.to_s, xml.at_css("prerelease").content
assert_equal @version.summary.to_s, xml.at_css("summary").content
assert_equal @version.licenses, xml.at_css("licenses").content
+ assert_equal @version.requirements, xml.at_css("requirements").content
end
end
Something went wrong with that request. Please try again.