Issue/165 #168
Issue/165 #168
Conversation
There was a problem hiding this comment.
🐳 👍 🐳
This looks good to me. I have minor comments but nothing blocking other than the stray code from #167
I think we can provide some more affordances around the nodejs dependency such as the option of docker based runs for node-based tooling but since this is not a requirement I think it can be done in a later PR.
One caveat is that I don't use Typescript often and I don't use an editor that integrates with these annotations so I can't speak for the effectiveness/correctness of the actual editor stuff.
| @@ -0,0 +1 @@ | |||
| nodejs 20.5.1 | |||
There was a problem hiding this comment.
Yes! Love to see node version management with asdf available
| @@ -68,6 +68,7 @@ deployments/ contains files used for deployment technologies | |||
| docs/ contains documentation about the project | |||
| examples/ contains additional `Dockerfile` examples that extend the base | |||
| configuration | |||
There was a problem hiding this comment.
Are there any notes we should make about editor integration or docs generation? Do we need to add a note about the nodejs requirement?
| @@ -22,3 +22,13 @@ help: | |||
| .PHONY: test | |||
| test: ## Run all tests | |||
| $Q $(CURDIR)/test.sh --type oss --unprivileged false --latest-njs false | |||
|
|
|||
| out: | |||
There was a problem hiding this comment.
Two comments:
- You can probably use
npx jsdoc -crather than directly referencing the js file which could change out from under us. outseems like a rather generic name for building the docs.docsmight be more intuitive?
| @@ -33,10 +47,16 @@ const NOW = new Date(); | |||
| const ECS_CREDENTIAL_BASE_URI = 'http://169.254.170.2'; | |||
|
|
|||
| /** | |||
| * URL to EC2 Instance Metadata Service (IMDS) token endpoint | |||
| * @see {@link https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html| EC2 Instance Metadata Service} | |||
| @@ -322,9 +358,9 @@ function redirectToS3(r) { | |||
|
|
|||
| if (isDirectoryListing && (r.method === 'GET' || r.method === 'HEAD')) { | |||
| r.internalRedirect("@s3PreListing"); | |||
| } else if ( PROVIDE_INDEX_PAGE == true ) { | |||
| } else if (PROVIDE_INDEX_PAGE === true) { | |||
There was a problem hiding this comment.
These changes seem like they are from this PR. Might want to remove them from or note that this PR is dependent on that one being merged first and a rebase on this one.
|
added some changes and tested in VSC. and submitted a PR dekobon#1 to share my findings |
Co-authored-by: Maxim Ivanitskiy <[email protected]>
This change adds support for building JSDoc docs using nodejs. It also improves the JSDoc annotations used in the njs source code.
The rationale for this change is that by improving JSDocs, you improve the static code analysis experience when using an IDE to work on njs code.
This change incorporates the following:
package.jsonwhich references the TypeScript types for internal njs types, jsdoc, and jsdoc dependencies (taffydb)