-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Conversation
|
There is Bundler code that is bening tested with |
|
I'm okay to merge this. But The 15000+ gems still uses We need to add the migration step that is the only |
|
@hsbt thanks for the feedback and @bronzdoc thank you for the review as well. What I will look to do is keep the removals but utilize Since we have access to
|
|
I will see how the checks go, I have reincluded an |
|
|
||
| data = Marshal.load Gem::Util.inflate(Gem.read_binary(path)) | ||
|
|
||
| assert_nil data.rubyforge_project |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.....
|
I am guessing failures like this: https://travis-ci.org/rubygems/rubygems/jobs/442597902 will require bundler changes. Failures like this: https://travis-ci.org/rubygems/rubygems/jobs/442597895 will require further understanding about what the test is looking to achieve. Open to thoughts on this or a decision if we can greenlight its removal. |
| 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] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Okay, I'll redo the offsets. It looks like the bundler failure (referenced
in another PR) is due to it tying to an older version of `rake` which still
uses `rubyforge_project` and they won't be able to bump it until `bundler
2.0`.
What I may do to 'get this over the line' is keep offsets the same, add a
note, and save adding the `deprecate` line on the `attr_writer` until
bundler accepts my bump PR in the future.
…On Wed, 24 Oct 2018 at 20:19, Ellen Marie Dash ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/rubygems/specification.rb
<#2436 (comment)>:
> @@ -1349,16 +1348,15 @@ def self._load(str)
spec.instance_variable_set ***@***.***_rubygems_version, array[7]
spec.instance_variable_set ***@***.***_platform, array[8]
spec.instance_variable_set ***@***.***, array[9]
- spec.instance_variable_set ***@***.***_project, array[10]
👍 for keeping the offsets the same. You should probably also add a note
explaining that it's deprecated/no longer used.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2436 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHgCxmndAW4jMUmKdFqYud0-PvQZ1GSWks5uoL0qgaJpZM4XcXcY>
.
|
|
Should have something up monday -just replying from email
On Thu, 25 Oct 2018 at 15:41, Nicholas Schwaderer <
[email protected]> wrote:
… Okay, I'll redo the offsets. It looks like the bundler failure (referenced
in another PR) is due to it tying to an older version of `rake` which still
uses `rubyforge_project` and they won't be able to bump it until `bundler
2.0`.
What I may do to 'get this over the line' is keep offsets the same, add a
note, and save adding the `deprecate` line on the `attr_writer` until
bundler accepts my bump PR in the future.
On Wed, 24 Oct 2018 at 20:19, Ellen Marie Dash ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In lib/rubygems/specification.rb
> <#2436 (comment)>:
>
> > @@ -1349,16 +1348,15 @@ def self._load(str)
> spec.instance_variable_set ***@***.***_rubygems_version, array[7]
> spec.instance_variable_set ***@***.***_platform, array[8]
> spec.instance_variable_set ***@***.***, array[9]
> - spec.instance_variable_set ***@***.***_project, array[10]
>
> 👍 for keeping the offsets the same. You should probably also add a note
> explaining that it's deprecated/no longer used.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#2436 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AHgCxmndAW4jMUmKdFqYud0-PvQZ1GSWks5uoL0qgaJpZM4XcXcY>
> .
>
|
|
Update - reference this in bundler and will do another PR in future with the full deprecation line so that we can move forward without breaking tests. Will make that change after bundler 2.0 Current changes restored offsets and removed explicit deprecation. |
|
So @Schwad, I take it this is ready to go now, right? Great job here, I think there's some other attributes where we could do the same thing, for example, |
|
@deivid-rodriguez just had a look, yes we should be good to go for this! |
|
@bundlerbot r+ |
2436: Remove rubyforge_page functionality r=bronzdoc a=Schwad # Description: Type: Cleanup RubyForge was [sunsetted in 2013](https://twitter.com/evanphx/status/399552820380053505) . There are still mentions of it in the code base. For this PR, instead of looking for dead links I aimed to clear out the `rubyforge_project` attr_accessor from the rubygems specification and the places that reference it. Perhaps there could be a conversation here whether something could 'replace', `rubyforge_project` (`github_project`?) but to start here is the 'lite' approach that simply removes it. Looking forward to thoughts and feedback! :) ______________ # Tasks: - [x] Describe the problem / feature - [ ] Write tests __(n/a)__ - [x] Write code to solve the problem - [x] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). 2511: Better unrestricted warning r=bronzdoc a=deivid-rodriguez # Description: This is a possible fix for #2508. If the dependency is fully unrestricted, give a generic message instead of guessing what the user wants, since we cannot really know it. ______________ # Tasks: - [x] Describe the problem / feature - [x] Write tests - [x] Write code to solve the problem - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). Co-authored-by: Nick Schwaderer <[email protected]> Co-authored-by: Nick Schwaderer <[email protected]> Co-authored-by: David Rodríguez <[email protected]>
Build succeeded |
Rubyforge was closed down in 2013 [1]. The rubyforge_project property was deprecated in rubygems in pull request 2436 [2]. [1] https://twitter.com/evanphx/status/399552820380053505 [2] ruby/rubygems#2436
RubyForge was closed down in 2013 [1]. The `rubyforge_project` property was deprecated in rubygems in pull request 2436 [2]. [1] https://twitter.com/evanphx/status/399552820380053505 [2] ruby/rubygems#2436
RubyForge was closed down in 2013 [1]. The `rubyforge_project` property was deprecated in rubygems in pull request 2436 [2]. [1] https://twitter.com/evanphx/status/399552820380053505 [2] ruby/rubygems#2436
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. Refer to ruby/rubygems#2436
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. Refer to ruby/rubygems#2436
RubyForge was closed down in 2013 [1]. The `rubyforge_project` property was deprecated in rubygems in pull request 2436 [2]. [1] https://twitter.com/evanphx/status/399552820380053505 [2] ruby/rubygems#2436
RubyForge was sunsetted in 2013. ref. ruby/rubygems#2436
The RubyGems gemspec property rubyforge_project has been removed without a replacement. ## Background * [RubyForge was closed down in 2013][1]. * [ruby/rubygems#2436 deprecated the `rubyforge_project` property][2]. [1]: https://twitter.com/evanphx/status/399552820380053505 [2]: ruby/rubygems#2436
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. Refer to ruby/rubygems#2436
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. Refer to ruby/rubygems#2436
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. This PR removes that property. Background: - RubyForge was closed down in 2013, https://twitter.com/evanphx/status/399552820380053505 - ruby/rubygems#2436 deprecated the `rubyforge_project` property
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. This PR removes that property. Background: - RubyForge was closed down in 2013, https://twitter.com/evanphx/status/399552820380053505 - ruby/rubygems#2436 deprecated the `rubyforge_project` property
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. Refer to ruby/rubygems#2436
The RubyGems gemspec property `rubyforge_project` has been removed without a replacement. This PR removes that property. Background: - RubyForge was closed down in 2013, https://twitter.com/evanphx/status/399552820380053505 - ruby/rubygems#2436 deprecated the `rubyforge_project` property
Description:
Type: Cleanup
RubyForge was sunsetted in 2013 . There are still mentions of it in the code base. For this PR, instead of looking for dead links I aimed to clear out the
rubyforge_projectattr_accessor from the rubygems specification and the places that reference it.Perhaps there could be a conversation here whether something could 'replace',
rubyforge_project(github_project?) but to start here is the 'lite' approach that simply removes it.Looking forward to thoughts and feedback! :)
Tasks:
I will abide by the code of conduct.