Skip to content

util: Remove p, has been deprecated for years#2529

Closed
geek wants to merge 1 commit intonodejs:masterfrom
geek:deprecate-p
Closed

util: Remove p, has been deprecated for years#2529
geek wants to merge 1 commit intonodejs:masterfrom
geek:deprecate-p

Conversation

@geek
Copy link
Member

@geek geek commented Aug 24, 2015

This function has been deprecated for years... it's not documented either, so should fall under the implicit API deprecation policy:

https://github.com/joyent/node/blob/v0.8.28-release/lib/util.js#L450-L454

@brendanashworth brendanashworth added util Issues and PRs related to the built-in util module. semver-major PRs that contain breaking changes and should be released in the next major version. labels Aug 24, 2015
@chrisdickinson
Copy link
Contributor

chrisdickinson commented Aug 24, 2015

Adding this to the list of things to check.

@brendanashworth
Copy link
Contributor

brendanashworth commented Aug 24, 2015

+1, FWIW, here's the deprecation commit from five years ago: 022c083

@Fishrock123
Copy link
Contributor

Fishrock123 commented Aug 24, 2015

I think it's definitely safe to remove something deprecated in v0.1.96 hahaha

@mscdex
Copy link
Contributor

mscdex commented Aug 25, 2015

LGTM

@geek
Copy link
Member Author

geek commented Aug 31, 2015

@chrisdickinson update?

@rvagg rvagg mentioned this pull request Sep 16, 2015
Closed
@chrisdickinson
Copy link
Contributor

chrisdickinson commented Sep 16, 2015

Still working on the tool to check. It's coming along, but I'm not sure there should be a rush to remove this?

@chrisdickinson
Copy link
Contributor

chrisdickinson commented Sep 16, 2015

(OTOH, it's really unlikely that removing .p will break anything — I just lean towards the side of paranoia.)

@bnoordhuis
Copy link
Member

bnoordhuis commented Sep 16, 2015

For the record, util.p was deprecated almost five years ago in commit e38eb0c. It's been printing a deprecation warning since node v0.3.0.

@rvagg rvagg mentioned this pull request Sep 23, 2015
Closed
@rvagg
Copy link
Member

rvagg commented Sep 23, 2015

TSC agreed to remove util.p in master, I believe this can be merged but @Fishrock123 has the action item for this

@rvagg rvagg removed the tsc-agenda label Sep 23, 2015
@targos targos added this to the 5.0.0 milestone Oct 9, 2015
@targos
Copy link
Member

targos commented Oct 9, 2015

Added to the 5.0.0 milestone.
ping @Fishrock123

@Fishrock123
Copy link
Contributor

Fishrock123 commented Oct 10, 2015

LGTM

@thefourtheye
Copy link
Contributor

thefourtheye commented Oct 10, 2015

LGTM.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 10, 2015

Quick grep results for (sys|util)\.p\(:

deck-node-1.0.11.tgz/typed/async/async-tests.ts:185:    function () { sys.p('one'); },
deck-node-1.0.11.tgz/typed/async/async-tests.ts:186:    function () { sys.p('two'); },
deck-node-1.0.11.tgz/typed/async/async-tests.ts:187:    function () { sys.p('three'); }
definitively-typed-0.0.1.tgz/async/async-tests.ts:243:    function () { sys.p('one'); },
definitively-typed-0.0.1.tgz/async/async-tests.ts:244:    function () { sys.p('two'); },
definitively-typed-0.0.1.tgz/async/async-tests.ts:245:    function () { sys.p('three'); }
mysql-native-prerelease-1.4.2.tgz/examples/myhttp.js:8:process.addListener('uncaughtException', function(err) { sys.p(err); });
mysql-native-prerelease-1.4.2.tgz/examples/myhttp.js:40:  sys.p(q);
mysql-native-prerelease-1.4.2.tgz/tests/test_execute.js:11:examplecmd.on('error', function(s) { sys.p(s); } );
mysql-native-prerelease-1.4.2.tgz/tests/test_stress.js:9:sys.p(numclients);
noblerecord-v1.0.1.tgz/src/mysql.js:154:                sys.p(me.connection.connectError);
restler-aaronblohowiak-0.0.2.tgz/test/multipartform.js:37:             sys.p(bytesWritten);
restler-aaronblohowiak-0.0.2.tgz/test/multipartform.js:72:     sys.p("closing and sending");
shoutcast-0.0.2.tgz/lib/file.js:23:                sys.p(erro);
webidl.js-0.1.0.tgz/scratch/test.js:22:    sys.p(e);

LGTM

@ChALkeR ChALkeR mentioned this pull request Oct 10, 2015
Closed
@rvagg rvagg mentioned this pull request Oct 16, 2015
Closed
13 tasks
@targos
Copy link
Member

targos commented Oct 16, 2015

CI before landing: https://ci.nodejs.org/job/node-test-commit/861/

@targos
Copy link
Member

targos commented Oct 16, 2015

@geek this breaks a test. Do you have time to fix it ?

@targos targos mentioned this pull request Oct 19, 2015
Merged
@targos
Copy link
Member

targos commented Oct 19, 2015

#3432

targos pushed a commit to targos/node that referenced this pull request Oct 19, 2015
@geek @targos
util: Remove p, has been deprecated for years
8b4adb2 
Update deprecation test to use another method.

Ref: nodejs#2529
PR-URL: nodejs#3432
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@targos
Copy link
Member

targos commented Oct 19, 2015

Landed in 8b4adb2.

@targos targos closed this Oct 19, 2015
geek added a commit that referenced this pull request Oct 21, 2015
@geek @rvagg
util: Remove p, has been deprecated for years
412252c 
Update deprecation test to use another method.

Ref: #2529
PR-URL: #3432
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

5.0.0

Development

Successfully merging this pull request may close these issues.

10 participants

@geek @chrisdickinson @brendanashworth @Fishrock123 @mscdex @bnoordhuis @rvagg @targos @thefourtheye @ChALkeR