We should just deprecate it in v7, if anyone is actually going to yell about this I'd prefer to know now rather than in 6 months.

@nodejs/ctc ... please weigh in on this!

This should go through a proper deprecation cycle

@jasnell Did your deprecation policy ever land?

If we don't have a deprecation policy, let's get one.

If we have a deprecation policy, we should follow it. And if we feel like there are situations where it should be ignored, then we should change it. It shouldn't be treated as an advisory document. It should be treated as a contract with Node.js users.

Nope, not yet, but we need to get going on it.

No traces of NODE_TTY_UNSAFE_ASYNC in js/coffee/ts code from npm, as of 2016-11-24.

Also, https://github.com/search?l=JavaScript&q=NODE_TTY_UNSAFE_ASYNC&type=Code doesn't show any real uses.

There is virtually no difference in guaranteed behaviour here, so removing the env var barely qualifies as semver-major in my eyes.

given @ChALkeR's search results, I'm +1 on getting the deprecation into v7 and removing in v8.

I don't think usage of runtime env variables in JS files is a good metric. It's the kind of options that you will more likely find in deployment scripts or config files.

That said, I'm also +1 on removing in v8.

There is virtually no difference in guaranteed behaviour here, so removing the env var barely qualifies as semver-major in my eyes.

We could "remove" it in a minor, but it would have to be a no-op.

We could "remove" it in a minor, but it would have to be a no-op.

I’m not sure I understand… is there a difference between making the flag a no-op and just removing any behaviour that was behind it?

I’m not sure I understand… is there a difference between making the flag a no-op and just removing any behaviour that was behind it?

@addaleax Yeah, if we don't parse the flag node will exit if it is passed. :/

But that's not a flag. It's just an environment variable.

Oh, of course. Whoops >_<

I'm +1 for removing it, I just am unsure if it should be removed in v7.x as opposed to waiting for v8.x. To be on the safe side, I think we should wait until v8.x to remove, but possibly add a runtime deprecation to it in v7.x? We definitely need a way to communicate changes that we want to make like this to the ecosystem without disruption. This one is particularly difficult due to this being something that would probably be more used in services more so than in packages (imo).

I'm +1 on getting the deprecation into v7 and removing in v8.

+1 on removing.

There is virtually no difference in guaranteed behaviour here, so removing the env var barely qualifies as semver-major in my eyes.

Agreed.

I'd still prefer for this to go through a proper deprecation cycle but I'll switch my objection to a -0.

I would also prefer this to go through a proper deprecation cycle, so I'm -1 on this change.

As @evanlucas said better than I could:

We definitely need a way to communicate changes that we want to make like this to the ecosystem without disruption. This one is particularly difficult due to this being something that would probably be more used in services more so than in packages (imo).

Honestly, adding a runtime deprecation would be so much more disruptive than just dropping this feature completely…

And I’m not sure it’s clear to everyone, but just in case, I’ll expand on my comment above:
Node will attempt synchronous writes with or without the env var being present, and whether that succeeds or it has to fallback to writing asynchronously virtually always depends on external factors (e.g. performance of the terminal emulator); which means that one can’t really get reliable behaviour out of this feature anyways.

I agree with @addaleax' last comment, so +1 from me, however I would prefer more if we just did a proper deprecation cycle and I don't see how that could be more disruptive, we just add a check for the env var being set to '1' and use the deprecation message output and proceed as normal. That would be easy enough in 7.x and we could remove in 8.x, so an expedited deprecation cycle really.

@Fishrock123 ... how do you want to proceed on this one? Either way it needs a rebase

picked this up @ #12057

Will go ahead and close this in favor of the new PR