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.

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

Revert "Exclude common core dump file names by default" #38

Closed
wants to merge 1 commit into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Jul 3, 2019

This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
also a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".

@addaleax, I'd love to discuss how best to address the original problem that led to #8 being created. I think we can fix it in a way that doesn't require so many workarounds.

This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
_also_ a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".
isaacs added a commit that referenced this pull request Oct 9, 2019
This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
_also_ a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".

PR-URL: #38
Credit: @isaacs
Close: #38
Reviewed-by: @isaacs
isaacs added a commit that referenced this pull request Oct 9, 2019
This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
_also_ a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".

PR-URL: #38
Credit: @isaacs
Close: #38
Reviewed-by: @isaacs
isaacs added a commit that referenced this pull request Oct 9, 2019
This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
_also_ a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".

PR-URL: #38
Credit: @isaacs
Close: #38
Reviewed-by: @isaacs
isaacs added a commit that referenced this pull request Oct 9, 2019
This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
_also_ a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".

PR-URL: #38
Credit: @isaacs
Close: #38
Reviewed-by: @isaacs
isaacs added a commit that referenced this pull request Oct 9, 2019
This reverts commits 1cd776c and
800f03d.

While it's a great idea to not publish core dumps to the npm registry,
this approach is too blunt, and has broken a lot of packages in the
process.

Attempts to restrict the core dump file handling to avoid excluding
folders ends up including anything in any folder named core, which is
_also_ a problem.

An optimal solution would look at the actual files along the walk, and
filter them out if they're actually core dumps, using a more reliable
indicator than the filename "core".

PR-URL: #38
Credit: @isaacs
Close: #38
Reviewed-by: @isaacs
@isaacs
Copy link
Contributor Author

isaacs commented Oct 10, 2019

This is included in #40

@isaacs isaacs closed this Oct 10, 2019
@lukekarrys lukekarrys deleted the revert-pr-8 branch August 18, 2022 04:09
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.

1 participant