src: always use diagnostic file sequence number by cjihrig · Pull Request #27142 · nodejs/node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation6 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This commit attaches a sequence number to all filenames that are automatically generated by DiagnosticFilename. This prevents accidental overwriting of existing files.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

nodejs-github-bot added c++

Issues and PRs that require attention from people who are familiar with C++.

report

Issues and PRs related to process.report.

labels

Apr 8, 2019

Choose a reason for hiding this comment

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

This makes the sequence number shared across all uses of MakeFilename(), including those that use different prefix's and/or ext's. I'm okay with it (although it feels a bit odd) but I'll point it out in case others feel differently.

This makes the sequence number shared across all uses of MakeFilename(), including those that use different prefix's and/or ext's.

Yes, I was aware of that as well. I don't think we need to overcomplicate this. If someone wants a custom filename, they can pass in a custom filename. Thanks for pointing it out though.

Choose a reason for hiding this comment

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

LGTM

BridgeAR added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Apr 9, 2019

This commit attaches a sequence number to all filenames that are automatically generated by DiagnosticFilename. This prevents accidental overwriting of existing files.

PR-URL: nodejs#27142 Reviewed-By: Anna Henningsen anna@addaleax.net Reviewed-By: Richard Lau riclau@uk.ibm.com Reviewed-By: Michael Dawson michael_dawson@ca.ibm.com Reviewed-By: James M Snell jasnell@gmail.com Reviewed-By: Ruben Bridgewater ruben@bridgewater.de

This was referenced

Apr 23, 2019

Reviewers

jasnell jasnell approved these changes