Skip to content

Remove unused method RDoc::Require#top_level that always raises NoMethodError - #1813

Merged
tompng merged 1 commit into
ruby:masterfrom
tompng:rm_require_deadcode
Sep 7, 2026
Merged

Remove unused method RDoc::Require#top_level that always raises NoMethodError#1813
tompng merged 1 commit into
ruby:masterfrom
tompng:rm_require_deadcode

Conversation

@tompng

@tompng tompng commented Sep 6, 2026

Copy link
Copy Markdown
Member

It has been broken since 2d88424 (Move global parse state to RDoc::Store) removed TopLevel.all_files_hash and TopLevel.all_files.

Result of RDoc::Require.new('a','b').top_level is basically unchanged.
Before: NoMethodError(undefined method 'all_files_hash')
After: NoMethodError(undefined method 'top_level')

Copilot AI balanced review requested due to automatic review settings September 6, 2026 06:19
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Documentation preview

View the preview

Commit: f467787

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The removed method is broken and has no remaining callers in the repository.

Pull request overview

Removes a long-broken, unused RDoc::Require#top_level API and its obsolete state.

Changes:

  • Removes the unused @top_level initialization.
  • Removes the method referencing deleted RDoc::TopLevel APIs.
File summaries
File Description
lib/rdoc/code_object/require.rb Removes obsolete top-level lookup logic.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/rdoc/code_object/require.rb Outdated
@@ -16,7 +16,6 @@ class Require < CodeObject
def initialize(name, comment)
super()
@name = name.gsub(/'|"/, "") #'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not related, but does the #' here does anything?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed.
It's strange. There's two more #' comments in lib/rdoc/context.rb 🤔

…hodError

It has been broken since 2d88424 (Move global parse state to RDoc::Store)
removed TopLevel.all_files_hash and TopLevel.all_files.

Result of `RDoc::Require.new('a','b').top_level` is basically unchanged.
Before: NoMethodError(undefined method 'all_files_hash')
After: NoMethodError(undefined method 'top_level')
Copilot AI review requested due to automatic review settings September 7, 2026 12:37
@tompng
tompng force-pushed the rm_require_deadcode branch from e37929e to f467787 Compare September 7, 2026 12:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The removed method is nonfunctional and has no remaining repository references.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@tompng
tompng merged commit 20fa463 into ruby:master Sep 7, 2026
29 checks passed
@tompng
tompng deleted the rm_require_deadcode branch September 7, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants