GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: refactor constants, "deprecate" require('constants') by jasnell · Pull Request #6534 · nodejs/node

Conversation

@jasnell

@jasnell jasnell commented May 2, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

constants, os, fs, crypto

Description of change

The require('constants') module is currently undocumented and mashes together unrelated constants. This soft deprecates the require('constants') in favor of distinct os.constants, fs.constants, and crypto.constants that are specific to the modules for which they are relevant. The next step is to document those within the specific modules.

The justification for this is straightforward:

  1. The constants module was never actually documented. It's referenced a number of times in documentation but never adequately supported.
  2. The single constants object mashes up a number of unrelated constants, which can cause issues with sloppy code. For instance, a bug in spawnSync (https://github.com/nodejs/node/blob/master/lib/child_process.js#L383-L391) essentially allows any defined constant to be used as a signal. In another instance, the code is implementing hacks (https://github.com/nodejs/node/blob/master/lib/internal/process.js#L171-L174) to limit look ups to just signals.
  3. Constants should be exported by the modules in which they are actually relevant, and they should be logically grouped so that they make sense.

On the downside, there's a slight bit more overhead in creating the process.binding('constants').

/cc @addaleax @nodejs/documentation @nodejs/ctc @nodejs/crypto

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@jasnell jasnell added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to Node.js documentation. fs Issues and PRs related to file-system APIs and the fs module. os Issues and PRs related to the os subsystem. labels May 2, 2016
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 2, 2016
@jasnell jasnell mentioned this pull request May 2, 2016
doc: add initial constants module documentation #6505
Closed
1 task
@jasnell jasnell added the wip Issues and PRs that are still a work in progress. label May 2, 2016
@jasnell

jasnell commented May 2, 2016

Copy link
Copy Markdown
Member Author

Marking this in-progress because additional work is required to fill out the documentation for constants

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@eljefedelrodeodeljefe

eljefedelrodeodeljefe commented May 2, 2016

Copy link
Copy Markdown
Contributor

Good idea. nit: Can you indent the table HTML in the docs?

jasnell reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@addaleax addaleax mentioned this pull request May 2, 2016
repl: preload missing constants module #6494
Closed
2 tasks done
@jasnell jasnell mentioned this pull request May 3, 2016
process: add O_NOATIME constant #6492
Closed
3 tasks done
@ChALkeR

ChALkeR commented May 3, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@jasnell What is the reason behind grouping them to .constants? Why not just module.CONSTANT?

vkurchatkin reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ChALkeR
ChALkeR reviewed May 3, 2016
View reviewed changes
Comment thread tools/eslint/node_modules/graceful-fs/polyfills.js Outdated

@ChALkeR ChALkeR May 3, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't think this file should be changed in this commit.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@jasnell jasnell May 3, 2016

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Whoops! Picked up a stray!
On May 3, 2016 1:34 AM, "Сковорода Никита Андреевич" <
[email protected]> wrote:

In tools/eslint/node_modules/graceful-fs/polyfills.js
#6534 (comment):

@@ -1,5 +1,4 @@
var fs = require('./fs.js')
-var constants = require('constants')

I don't think this file should be changed in this commit.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/6534/files/4c6c0c9b0898ef360b2ce648cf7cf41d3d22d560#r61851725

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

@ChALkeR ... I went with the {module}.constants approach largely because of how the constants are defined (in a process.binding) and because it helps isolate them. For instance, look at the os.constants.signals, we have code that validates signals currently by just checking to see if the signal string appears as a property on the constants object. The current code is pretty much a hack but this change isolates the signal constants on their own object so that the check can be done without the additional hacks. Also, I'd prefer not to muddy up the main module API with too many constants that may not be used extensively.

I have no problem with exposing the constants directly off the module itself but this way just seems cleaner and more natural to me.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fishrock123 Fishrock123 added the semver-major PRs that contain breaking changes and should be released in the next major version. label May 3, 2016
@Fishrock123

Fishrock123 commented May 3, 2016

Copy link
Copy Markdown
Contributor

Marking as major because the deprecation and pretty radical change. I've yet to take a detailed look, but I think I'm in favor of this approach.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

FWIW, given that require('constants') was never actually documented and given the fact that there is no hard deprecation, and require('constants') continues to work as it did before, it would likely be ok for this to be a minor. In fact, we could simply choose not say anything about deprecating require('constants').

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell
jasnell force-pushed the more-logical-constants branch from 4c6c0c9 to e791429 Compare May 3, 2016 16:23
@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

@ChALkeR .. updated to remove the errant tool dependency change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ChALkeR

ChALkeR commented May 3, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@Fishrock123 There is no actual deprecation here, this looks like a semver-minor to me.

As there are no docs, the next step would be to print a warning on require('constants'), that one would be a semver-major.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

@ChALkeR ... can you do a quick scan of the modules to see how many hits you get for require('constants'). In the initial version of this I had a hard deprecation for require('constants') and just running lint caused a bunch of warnings because of dependencies inside eslint. I'm thinking that if we did ever hard deprecate require('constants') we'd want to wait a while and give some heads up first.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ChALkeR

ChALkeR commented May 3, 2016

Copy link
Copy Markdown
Member

@jasnell That was already done in #6494 (comment).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ChALkeR

ChALkeR commented May 3, 2016

Copy link
Copy Markdown
Member

@jasnell The hard deprecation clearly should not happen in the next major (or even in 8.0), because that will require all those modules (like npm) to utilize feature detection.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ChALkeR

ChALkeR commented May 3, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I would prefer graceful-fs to include feature detection there asap, though — just too many modules are affected through it.
It has about 50% of all downloads with require('constants').

It should be as trivial as var constants = fs.constants || require('constants').

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fishrock123
Fishrock123 force-pushed the master branch from 05f4313 to eb12f93 Compare May 3, 2016 16:46
@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

yeah, that's what I was afraid of. Ok, let's keep it as a soft deprecation for a long time then :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ChALkeR

ChALkeR commented May 3, 2016

Copy link
Copy Markdown
Member

@jasnell I wouldn't call this a soft deprecation — there are no docs for constants to deprecate them =).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

Yes, but there are at least a couple of existing mentions of require('constants') tho in a couple of places even if the module itself is not explicitly documented. I dunno... I'm perfectly happy with this as a semver-minor because the chance of it breaking anything is pretty small.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell

jasnell commented May 3, 2016

Copy link
Copy Markdown
Member Author

btw, CI: https://ci.nodejs.org/job/node-test-pull-request/2483/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell

jasnell commented May 17, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

hmm.. getting some issues with the new N_NOATIME constant on linux... investigating now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell

jasnell commented May 17, 2016

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell
src: refactor require('constants')
d52bc0c
The require('constants') module is currently undocumented and mashes
together unrelated constants. This refactors the require('constants')
in favor of distinct os.constants, fs.constants, and crypto.constants
that are specific to the modules for which they are relevant. The
next step is to document those within the specific modules.
@jasnell
jasnell force-pushed the more-logical-constants branch from 187a525 to d52bc0c Compare May 17, 2016 17:37
@jasnell

jasnell commented May 17, 2016

Copy link
Copy Markdown
Member Author

CI is green except for unrelated failure.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell added a commit that referenced this pull request May 17, 2016
@jasnell
src: refactor require('constants')
dcccbfd
The require('constants') module is currently undocumented and mashes
together unrelated constants. This refactors the require('constants')
in favor of distinct os.constants, fs.constants, and crypto.constants
that are specific to the modules for which they are relevant. The
next step is to document those within the specific modules.

PR-URL: #6534
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Robert Lindstaedt <[email protected]>
@jasnell

jasnell commented May 17, 2016

Copy link
Copy Markdown
Member Author

Landed in dcccbfd

eljefedelrodeodeljefe, ChALkeR, and bl-ue reacted with hooray emoji
All reactions
  • 🎉 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jasnell jasnell closed this May 17, 2016
jasnell added a commit to jasnell/node that referenced this pull request Jun 8, 2016
@jasnell
zlib: move constants into zlib.constants
c418104
zlib constants were previously being added to binding in node_zlib.cc.
This moves the zlib constants to node_constants.cc for consistency with
the recent constants refactoring:
  nodejs#6534

Adds require('zlib').constants to expose the constants
Docs-only deprecates the constants hung directly off require('zlib')
Removes a couple constants from the docs that apparently no longer
exist in the code
jasnell added a commit that referenced this pull request Jun 12, 2016
@jasnell
zlib: move constants into zlib.constants
197a465
zlib constants were previously being added to binding in node_zlib.cc.
This moves the zlib constants to node_constants.cc for consistency with
the recent constants refactoring:
  #6534

Adds require('zlib').constants to expose the constants
Docs-only deprecates the constants hung directly off require('zlib')
Removes a couple constants from the docs that apparently no longer
exist in the code

PR-URL: #7203
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@lance lance mentioned this pull request Jun 16, 2016
doc: Clarify fs.access works on directories too. #7113
Closed
2 tasks done
Fishrock123 pushed a commit that referenced this pull request Jun 27, 2016
@jasnell @Fishrock123
src: refactor require('constants')
bfcc650
The require('constants') module is currently undocumented and mashes
together unrelated constants. This refactors the require('constants')
in favor of distinct os.constants, fs.constants, and crypto.constants
that are specific to the modules for which they are relevant. The
next step is to document those within the specific modules.

PR-URL: #6534
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Robert Lindstaedt <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Jun 27, 2016
Propose v6.3.0 #7441
Merged
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
@jasnell @Fishrock123
src: refactor require('constants')
929b6c2
The require('constants') module is currently undocumented and mashes
together unrelated constants. This refactors the require('constants')
in favor of distinct os.constants, fs.constants, and crypto.constants
that are specific to the modules for which they are relevant. The
next step is to document those within the specific modules.

PR-URL: #6534
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Robert Lindstaedt <[email protected]>

 Conflicts:
	doc/api/fs.md
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
Propose v6.3.0 (v2) #7550
Merged
Fishrock123 added a commit that referenced this pull request Jul 6, 2016
@Fishrock123
2016-07-06, Version 6.3.0 (Current)
50aede5
Notable changes:

* buffer: Added `buffer.swap64()` to compliment `swap16()` &
`swap32()`. (Zach Bjornson) #7157
* build: New `configure` options have been added for building Node.js
as a shared library. (Stefan Budeanu)
#6994
  - The options are: `--shared`, `--without-v8-platform` &
`--without-bundled-v8`.
* crypto: Root certificates have been updated. (Ben Noordhuis)
#7363
* debugger: The server address is now configurable via
`--debug=<address>:<port>`. (Ben Noordhuis)
#3316
* npm: Upgraded npm to v3.10.3 (Kat Marchán)
#7515 & (Rebecca Turner)
#7410
* readline: Added the `prompt` option to the readline constructor.
(Evan Lucas) #7125
* repl / vm: `sigint`/`ctrl+c` will now break out of infinite loops
without stopping the Node.js instance. (Anna Henningsen)
#6635
* src:
  - Added a `node::FreeEnvironment` public C++ API. (Cheng Zhao)
#3098
  - Refactored `require('constants')`, constants are now available
directly from their respective modules. (James M Snell)
#6534
* stream: Improved `readable.read()` performance by up to 70%. (Brian
White) #7077
* timers: `setImmediate()` is now up to 150% faster in some situations.
(Andras) #6436
* util: Added a `breakLength` option to `util.inspect()` to control how
objects are formatted across lines. (cjihrig)
#7499
* v8-inspector: Experimental support has been added for debugging
Node.js over the inspector protocol. (Ali Ijaz Sheikh)
#6792
  - *Note: This feature is experimental, and it could be altered or
removed.*
  - You can try this feature by running Node.js with the `--inspect`
flag.

Refs: #7441
PR-URL: #7550
Fishrock123 added a commit that referenced this pull request Jul 6, 2016
@Fishrock123
2016-07-06, Version 6.3.0 (Current)
9fc01b1
Notable changes:

* buffer: Added `buffer.swap64()` to compliment `swap16()` &
`swap32()`. (Zach Bjornson) #7157
* build: New `configure` options have been added for building Node.js
as a shared library. (Stefan Budeanu)
#6994
  - The options are: `--shared`, `--without-v8-platform` &
`--without-bundled-v8`.
* crypto: Root certificates have been updated. (Ben Noordhuis)
#7363
* debugger: The server address is now configurable via
`--debug=<address>:<port>`. (Ben Noordhuis)
#3316
* npm: Upgraded npm to v3.10.3 (Kat Marchán)
#7515 & (Rebecca Turner)
#7410
* readline: Added the `prompt` option to the readline constructor.
(Evan Lucas) #7125
* repl / vm: `sigint`/`ctrl+c` will now break out of infinite loops
without stopping the Node.js instance. (Anna Henningsen)
#6635
* src:
  - Added a `node::FreeEnvironment` public C++ API. (Cheng Zhao)
#3098
  - Refactored `require('constants')`, constants are now available
directly from their respective modules. (James M Snell)
#6534
* stream: Improved `readable.read()` performance by up to 70%. (Brian
White) #7077
* timers: `setImmediate()` is now up to 150% faster in some situations.
(Andras) #6436
* util: Added a `breakLength` option to `util.inspect()` to control how
objects are formatted across lines. (cjihrig)
#7499
* v8-inspector: Experimental support has been added for debugging
Node.js over the inspector protocol. (Ali Ijaz Sheikh)
#6792
  - *Note: This feature is experimental, and it could be altered or
removed.*
  - You can try this feature by running Node.js with the `--inspect`
flag.

Refs: #7441
PR-URL: #7550
Fishrock123 added a commit that referenced this pull request Jul 6, 2016
@Fishrock123
2016-07-06, Version 6.3.0 (Current)
7628031
Notable changes:

* buffer: Added `buffer.swap64()` to compliment `swap16()` &
`swap32()`. (Zach Bjornson) #7157
* build: New `configure` options have been added for building Node.js
as a shared library. (Stefan Budeanu)
#6994
  - The options are: `--shared`, `--without-v8-platform` &
`--without-bundled-v8`.
* crypto: Root certificates have been updated. (Ben Noordhuis)
#7363
* debugger: The server address is now configurable via
`--debug=<address>:<port>`. (Ben Noordhuis)
#3316
* npm: Upgraded npm to v3.10.3 (Kat Marchán)
#7515 & (Rebecca Turner)
#7410
* readline: Added the `prompt` option to the readline constructor.
(Evan Lucas) #7125
* repl / vm: `sigint`/`ctrl+c` will now break out of infinite loops
without stopping the Node.js instance. (Anna Henningsen)
#6635
* src:
  - Added a `node::FreeEnvironment` public C++ API. (Cheng Zhao)
#3098
  - Refactored `require('constants')`, constants are now available
directly from their respective modules. (James M Snell)
#6534
* stream: Improved `readable.read()` performance by up to 70%. (Brian
White) #7077
* timers: `setImmediate()` is now up to 150% faster in some situations.
(Andras) #6436
* util: Added a `breakLength` option to `util.inspect()` to control how
objects are formatted across lines. (cjihrig)
#7499
* v8-inspector: Experimental support has been added for debugging
Node.js over the inspector protocol. (Ali Ijaz Sheikh)
#6792
  - *Note: This feature is experimental, and it could be altered or
removed.*
  - You can try this feature by running Node.js with the `--inspect`
flag.

Refs: #7441
PR-URL: #7550
@cjean-fr cjean-fr mentioned this pull request Jul 7, 2016
Update nodejs to 6.3.0 ScoopInstaller/Scoop#948
Merged
lukesampson pushed a commit to ScoopInstaller/Scoop that referenced this pull request Jul 7, 2016
@cjean-fr @lukesampson
Update nodejs to 6.3.0 (#948)
3b8b000
### Notable changes

* **buffer**: Added `buffer.swap64()` to compliment `swap16()` & `swap32()`. (Zach Bjornson) [#7157](nodejs/node#7157)
* **build**: New `configure` options have been added for building Node.js as a shared library. (Stefan Budeanu) [#6994](nodejs/node#6994)
  - The options are: `--shared`, `--without-v8-platform` & `--without-bundled-v8`.
* **crypto**: Root certificates have been updated. (Ben Noordhuis) [#7363](nodejs/node#7363)
* **debugger**: The server address is now configurable via `--debug=<address>:<port>`. (Ben Noordhuis) [#3316](nodejs/node#3316)
* **npm**: Upgraded npm to v3.10.3 (Kat Marchán) [#7515](nodejs/node#7515) & (Rebecca Turner) [#7410](nodejs/node#7410)
* **readline**: Added the `prompt` option to the readline constructor. (Evan Lucas) [#7125](nodejs/node#7125)
* **repl / vm**: `sigint`/`ctrl+c` will now break out of infinite loops without stopping the Node.js instance. (Anna Henningsen) [#6635](nodejs/node#6635)
* **src**:
  - Added a `node::FreeEnvironment` public C++ API. (Cheng Zhao) [#3098](nodejs/node#3098)
  - Refactored `require('constants')`, constants are now available directly from their respective modules. (James M Snell) [#6534](nodejs/node#6534)
* **stream**: Improved `readable.read()` performance by up to 70%. (Brian White) [#7077](nodejs/node#7077)
* **timers**: `setImmediate()` is now up to 150% faster in some situations. (Andras) [#6436](nodejs/node#6436)
* **util**: Added a `breakLength` option to `util.inspect()` to control how objects are formatted across lines. (cjihrig) [#7499](nodejs/node#7499)
* **v8-inspector**: Experimental support has been added for debugging Node.js over the inspector protocol. (Ali Ijaz Sheikh) [#6792](nodejs/node#6792)
  - **Note: This feature is _experimental_, and it could be altered or removed.**
  - You can try this feature by running Node.js with the `--inspect` flag.
@ChALkeR ChALkeR mentioned this pull request Aug 28, 2016
node: deprecate public access to process.binding #2768
Closed
@wdavidw wdavidw mentioned this pull request Sep 1, 2016
FIX: update sftp after changes in latest nodejs constants mscdex/ssh2-streams#47
Closed
@addaleax addaleax mentioned this pull request Sep 27, 2016
question about implementation details isaacs/node-graceful-fs#48
Closed
@sam-github sam-github added doc Issues and PRs related to Node.js documentation. and removed doc Issues and PRs related to Node.js documentation. labels Dec 1, 2016
@tniessen tniessen added the deprecations Issues and PRs related to deprecations. label Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. deprecations Issues and PRs related to deprecations. doc Issues and PRs related to Node.js documentation. fs Issues and PRs related to file-system APIs and the fs module. os Issues and PRs related to the os subsystem. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

13 participants

@jasnell @eljefedelrodeodeljefe @ChALkeR @Fishrock123 @mscdex @Trott @addaleax @trevnorris @ronkorving @evanlucas @sam-github @tniessen @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.