Skip to content

Remove unused tempfile finalizer ivar sets - #42

Draft
HassanAkbar wants to merge 2 commits into
mainfrom
chore/todo-04-spec-ivars
Draft

Remove unused tempfile finalizer ivar sets#42
HassanAkbar wants to merge 2 commits into
mainfrom
chore/todo-04-spec-ivars

Conversation

@HassanAkbar

Copy link
Copy Markdown
Member

Metanorma PR checklist

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR removes a no-op Tempfile “finalizer suppression” workaround from the ZipExtractor specs (keeping the actual Windows-safe close+delete behavior) and updates the associated TODO note documenting the cleanup.

Changes:

  • Removed instance_variable_set(:@finalizer, ...) and Tempfile.remove_instance_variable usage from zip_extractor_spec.rb.
  • Updated the TODO entry describing the now-removed ivar hacks and current verification steps.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
TODO/04-reduce-spec-ivar-use.md Updates the project TODO note describing the “instance_variable_*” cleanup and verification guidance.
spec/uniword/infrastructure/zip_extractor_spec.rb Removes ineffective Tempfile “finalizer suppression” code while preserving the close+delete workaround for Windows file locking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread spec/uniword/infrastructure/zip_extractor_spec.rb Outdated
Comment thread TODO/04-reduce-spec-ivar-use.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

TODO/04-reduce-spec-ivar-use.md:41

  • This note hard-codes the "installed version" as tempfile-0.3.1, but the repo doesn’t pin tempfile (no lockfile) and CI/Ruby version changes will make that statement stale. The key point is the ivar name (@finalizer_manager), so it’d be clearer and more future-proof to avoid asserting a specific installed version.
**`Tempfile` has no `@finalizer` ivar.** The installed version
(`tempfile-0.3.1`) uses `@finalizer_manager`:

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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.

2 participants