Skip to content

Remove rubyforge_page functionality #2436

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.

Sign up for GitHub

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

Merged
10 commits merged into from
Dec 10, 2018
Merged

Remove rubyforge_page functionality #2436

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ed1ac63
Remove rubyforge_page functionality
Oct 15, 2018
dfe5fa6
Prevents gem breaking from rubyforge removal, makes deprecation vocal
Schwad Oct 16, 2018
0935f38
Point test to non-deprecated specification attribute
Oct 17, 2018
3dbfa8a
Merge branch 'master' of http://github.com/schwad/rubygems
Oct 17, 2018
6cbdf43
Deprecate rubyforge_project with same approach as has_rdoc
Oct 17, 2018
ef83406
Deprecate rubyforge_project as default_executable
Oct 17, 2018
bd9b003
Checks against default_executable attribute
Oct 17, 2018
d3bcc8f
Remove rubyforge deprecation, restore offset
Oct 25, 2018
57bc545
➕ :
Oct 25, 2018
a959a2f
empty string to preserve rubyforge_project
Oct 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions lib/rubygems/specification.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ class Gem::Specification < Gem::BasicSpecification
:required_ruby_version => Gem::Requirement.default,
:required_rubygems_version => Gem::Requirement.default,
:requirements => [],
:rubyforge_project => nil,
:rubygems_version => Gem::VERSION,
:signing_key => nil,
:specification_version => CURRENT_SPECIFICATION_VERSION,
Expand Down Expand Up @@ -729,15 +728,10 @@ def test_files= files # :nodoc:
##
# Allows deinstallation of gems with legacy platforms.

attr_writer :original_platform # :nodoc:
attr_writer :original_platform # :nodoc

##
# The rubyforge project this gem lives under. i.e. RubyGems'
# rubyforge_project is "rubygems".
#
# This option is deprecated.

attr_accessor :rubyforge_project
attr_writer :rubyforge_project # :nodoc
deprecate :rubyforge_project, :none, 2020, 4

##
# The Gem::Specification version of this gemspec.
Expand Down Expand Up @@ -1349,16 +1343,15 @@ def self._load(str)
spec.instance_variable_set :@required_rubygems_version, array[7]
spec.instance_variable_set :@original_platform, array[8]
spec.instance_variable_set :@dependencies, array[9]
spec.instance_variable_set :@rubyforge_project, array[10]
Copy link
Contributor

@segiddins segiddins Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change would require bumping the spec version -- it might be better to keep the offsets the same, and just never use the rubyforge ivar?

Copy link
Contributor

@duckinator duckinator Oct 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for keeping the offsets the same. You should probably also add a note explaining that it's deprecated/no longer used.

spec.instance_variable_set :@email, array[11]
spec.instance_variable_set :@authors, array[12]
spec.instance_variable_set :@description, array[13]
spec.instance_variable_set :@homepage, array[14]
spec.instance_variable_set :@has_rdoc, array[15]
spec.instance_variable_set :@new_platform, array[16]
spec.instance_variable_set :@platform, array[16].to_s
spec.instance_variable_set :@license, array[17]
spec.instance_variable_set :@metadata, array[18]
spec.instance_variable_set :@email, array[10]
spec.instance_variable_set :@authors, array[11]
spec.instance_variable_set :@description, array[12]
spec.instance_variable_set :@homepage, array[13]
spec.instance_variable_set :@has_rdoc, array[14]
spec.instance_variable_set :@new_platform, array[15]
spec.instance_variable_set :@platform, array[15].to_s
spec.instance_variable_set :@license, array[16]
spec.instance_variable_set :@metadata, array[17]
spec.instance_variable_set :@loaded, false
spec.instance_variable_set :@activated, false

Expand Down Expand Up @@ -1394,7 +1387,6 @@ def _dump(limit)
@required_rubygems_version,
@original_platform,
@dependencies,
@rubyforge_project,
@email,
@authors,
@description,
Expand Down
2 changes: 0 additions & 2 deletions test/rubygems/test_gem_commands_build_command.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def setup
end

@gem = util_spec 'some_gem' do |s|
s.rubyforge_project = 'example'
s.license = 'AGPL-3.0'
s.files = ['README.md']
end
Expand Down Expand Up @@ -97,7 +96,6 @@ def test_execute_strict_without_warnings

def test_execute_strict_with_warnings
bad_gem = util_spec 'some_bad_gem' do |s|
s.rubyforge_project = 'example'
s.files = ['README.md']
end

Expand Down
1 change: 0 additions & 1 deletion test/rubygems/test_gem_remote_fetcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class TestGemRemoteFetcher < Gem::TestCase
author: Jim Weirich
email: [email protected]
homepage: http://rake.rubyforge.org
rubyforge_project: rake
description: Rake is a Make-like program implemented in Ruby. Tasks and dependencies are specified in standard Ruby syntax.
autorequire:
default_executable: rake
Expand Down
13 changes: 0 additions & 13 deletions test/rubygems/test_gem_specification.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ def make_spec_c1
s.extensions << 'ext/a/extconf.rb'
s.test_file = 'test/suite.rb'
s.requirements << 'A working computer'
s.rubyforge_project = 'example'
s.license = 'MIT'

s.add_dependency 'rake', '> 0.4'
Expand Down Expand Up @@ -80,7 +79,6 @@ def setup
s.executable = 'exec'
s.test_file = 'test/suite.rb'
s.requirements << 'A working computer'
s.rubyforge_project = 'example'
s.license = 'MIT'

s.mark_version
Expand Down Expand Up @@ -701,7 +699,6 @@ def test_self_attribute_names
required_ruby_version
required_rubygems_version
requirements
rubyforge_project
rubygems_version
signing_key
specification_version
Expand Down Expand Up @@ -871,7 +868,6 @@ def test_self_from_yaml_cleans_up_Date_objects
author: Austin Ziegler
email: [email protected]
homepage: http://rubyforge.org/projects/ruwiki/
rubyforge_project: ruwiki
description: "Test"
bindir: bin
has_rdoc: true
Expand Down Expand Up @@ -1210,14 +1206,6 @@ def test_self_stubs_for_mult_platforms

DATA_PATH = File.expand_path "../data", __FILE__

def test_handles_private_null_type
path = File.join DATA_PATH, "null-type.gemspec.rz"

data = Marshal.load Gem::Util.inflate(Gem.read_binary(path))

assert_nil data.rubyforge_project
Copy link
Contributor

@simi simi Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just change assertion in here? What's purpose of this test?

Copy link
Contributor Author

@Schwad Schwad Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the history (blame) on this file and the latest commits on each line weren't too illuminating: https://github.com/rubygems/rubygems/blame/master/test/rubygems/test_gem_specification.rb

However nothing about this test makes me think that rubyforge_project is the necessary component. We can flag up some other authors but I could preserve this test with the assertion looking at a different attribute. (say 'description'). Will commit that now.

Copy link
Contributor

@simi simi Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's what I was thinking about as well. It is not clear what's the purpose of this test at all, but it is probably not directly testing rubyforge_project, but uses rubyforge_project to assert only. Changing assertion to use different attribute should work here also.

Copy link
Contributor Author

@Schwad Schwad Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly enough, as I just found out, description does not assert nil. I am wondering if this is only actually passing on an assert_nil because rubyforge_project simply isn't set anymore.....

end

def test_initialize
spec = Gem::Specification.new do |s|
s.name = "blah"
Expand Down Expand Up @@ -2496,7 +2484,6 @@ def test_to_ruby_fancy
s.homepage = "http://example.com".freeze
s.licenses = ["MIT".freeze]
s.requirements = ["A working computer".freeze]
s.rubyforge_project = "example".freeze
s.rubygems_version = "#{Gem::VERSION}".freeze
s.summary = "this is a summary".freeze
s.test_files = ["test/suite.rb".freeze]
Expand Down