Skip to content

Conversation

@W-A-James
Copy link
Contributor

@W-A-James W-A-James commented Mar 4, 2025

Description

What is changing?

  • move creation and removal of install directory to Suite.run from Task.run
  • Move tests for creation and removal of install directory to suite.test.ts from task.test.ts
Is there new documentation needed for these changes?

What is the motivation for this change?

Double check the following

  • Ran npm run check:eslint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@W-A-James W-A-James force-pushed the create-tmp-dir-per-suite branch from 4fa73bd to 0eefbd9 Compare March 4, 2025 18:41
@W-A-James W-A-James changed the title wip fix(NODE-6706): wip Mar 4, 2025
@W-A-James W-A-James changed the title fix(NODE-6706): wip fix(NODE-6706): Move creation and deletion of install location Mar 4, 2025
@W-A-James
Copy link
Contributor Author

Note that failing test only fails on [email protected]. This is similar to things we've seen in the past when making changes to this library where installing one or another version of bson suddenly stops working.

@W-A-James W-A-James marked this pull request as ready for review March 4, 2025 20:40
@W-A-James W-A-James requested a review from a team as a code owner March 4, 2025 20:40
@nbbeeken nbbeeken self-assigned this Mar 4, 2025
@nbbeeken nbbeeken added the Primary Review Pull request is being reviewed by primary reviewer label Mar 4, 2025
@nbbeeken nbbeeken merged commit c0bac58 into main Mar 5, 2025
1 of 6 checks passed
@nbbeeken nbbeeken deleted the create-tmp-dir-per-suite branch March 5, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Primary Review Pull request is being reviewed by primary reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants