Skip to content
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

Jump to bottom

Revert "module: fix check for package.json at volume root" #34403

Closed
wants to merge 1 commit into from
Closed

Revert "module: fix check for package.json at volume root" #34403

wants to merge 1 commit into from

Conversation

MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented Jul 16, 2020 •
edited
Loading

This reverts commit 51af89f.

This has needed to be backed out of both the 14.5.0 and 14.6.0
releases due to creating regressions across multiple projects
including:

  • coffeescript
  • JSONStream
  • gulp
  • and more

We should reopen a PR to figure out how to land this in a way
that is non-breaking.

Refs: #33476

@MylesBorins
Revert "module: fix check for package.json at volume root"
50e7859 
This reverts commit 51af89f.

This has needed to be backed out of both the 14.5.0 and 14.6.0
releases due to creating regressions across multiple projects
including:

* coffeescript
* JSONStream
* gulp
* and more

We should reopen a PR to figure out how to land this in a way
that is non-breaking.

Refs: nodejs#33476
@trivikr
Copy link
Member

trivikr commented Jul 18, 2020

Can you please share link to the regressions caused by the commit being reverted?

@DerekNonGeneric
Copy link
Contributor

Can you please share link to the regressions caused by the commit being reverted?

Yes, I would be interested in that as well. Is there a permalink for the aforementioned CITGM failures that justify this change?

@GeoffreyBooth
Copy link
Member

Can you please share link to the regressions caused by the commit being reverted?

#34093 (comment)

I hope to find time to fix this, but probably not for several days. If anyone has time to take a crack at redoing the reverted PR before then, please let us know so that we don't duplicate effort.

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented Jul 20, 2020 •
edited
Loading

If anyone has time to take a crack at redoing the reverted PR […]

A testing strategy for this would be very helpful. @GeoffreyBooth, I'll catch up w/ you in a few days to see about reworking that Docker strategy you came up with for the .mjs server stuff. Until then, this should obviously be reverted.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32411/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32414/

MylesBorins added a commit that referenced this pull request Jul 20, 2020
@MylesBorins
Revert "module: fix check for package.json at volume root"
da95dd7 
This reverts commit 51af89f.

This has needed to be backed out of both the 14.5.0 and 14.6.0
releases due to creating regressions across multiple projects
including:

* coffeescript
* JSONStream
* gulp
* and more

We should reopen a PR to figure out how to land this in a way
that is non-breaking.

Refs: #33476

PR-URL: #34403
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@MylesBorins
Copy link
Contributor Author

Landed in da95dd7

@github-actions github-actions bot mentioned this pull request Jul 21, 2020
Open
10 tasks
cjihrig pushed a commit that referenced this pull request Jul 23, 2020
@MylesBorins @cjihrig
Revert "module: fix check for package.json at volume root"
247ce2e 
This reverts commit 51af89f.

This has needed to be backed out of both the 14.5.0 and 14.6.0
releases due to creating regressions across multiple projects
including:

* coffeescript
* JSONStream
* gulp
* and more

We should reopen a PR to figure out how to land this in a way
that is non-breaking.

Refs: #33476

PR-URL: #34403
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This was referenced Jul 24, 2020
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@richardlau richardlau richardlau approved these changes

@guybedford guybedford guybedford approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@MylesBorins @trivikr @DerekNonGeneric @GeoffreyBooth @nodejs-github-bot @guybedford @richardlau @targos