Update for enjoi and tape packages, change dev dependencies. #97

dlobregon · 2021-03-08T16:57:26Z

No description provided.

…age-lock.json file

…file types

…ow a test case catch a path parameter

…sion

…json file

rtrys · 2021-03-08T17:55:21Z

lib/buildroutes.js

+/* eslint-disable global-require */
+/* eslint-disable no-use-before-define */
+/* eslint-disable prefer-const */
+/* eslint-disable max-len */


are those exceptions really needed?

I'm agree with you, the changes have been done.

rtrys · 2021-03-08T18:00:01Z

lib/buildroutes.js

+    utils.verbs.forEach((verb) => {
+      let route; let pathnames; let operation; let
+        validators;
+


looks like some of these variables can be const, and also can you use a line per declaration?

if one of them need to be let, can you declare it previous to first assignation?

You are right, I applied your suggestion using const and let.

rtrys · 2021-03-08T18:03:07Z

lib/buildroutes.js

-
-            route.handler && routes.push(route);
+  let api; let routes; let handlers; let defaulthandler; let
+    validator;


can you verify if is it possible to use const for these variables?

rtrys · 2021-03-08T18:07:17Z

package.json

@@ -17,24 +17,28 @@
  },
  "bugs": "http://github.com/krakenjs/swaggerize-builder/issues",
  "engines": {
-    "node": "<=4.x"
+    "node": ">=10"


should it be >=12?
latest version of Joi requires node 12

Thanks, the change is done.

tikochato · 2021-03-08T18:30:03Z

lib/validator.js

-        fragment = typeof fragment === 'object' && fragment[paths[i]];
-    }
+  // eslint-disable-next-line no-plusplus
+  for (let i = 1; i < paths.length && fragment; i++) {


Suggested change

for (let i = 1; i < paths.length && fragment; i++) {

for (let i = 1; i < paths.length && fragment; i+=1) {

We could use the i += 1 expression or we could add the "allowForLoopAfterthoughts": true rule to the eslint config file.

sure, the change is done.

…rules

RobertTheBlair

This solves issue 94 - please update that issue, bring it to the creator's attention

RobertTheBlair · 2021-03-10T01:20:02Z

CHANGELOG.md

@@ -1,3 +1,9 @@
+### 1.0.12
+
+- Update to node v10, add `package-lock.json` file


Should this be v12?

Might also want to make a bigger version jump if we are ruling out old node versions

+1 on Node 12. Maybe it can be changed to node 10+.
Curious, is package-lock.json for unit tests? Shouldn't unit tests always run on the latest dependency versions?

RobertTheBlair · 2021-03-10T05:26:06Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "swaggerize-routes",
-  "version": "1.0.11",
+  "version": "1.0.12",


is node 12 minimum a point release?

grawk · 2021-03-16T18:16:19Z

@dlobregon I've published 2.0.0-alpha.1 and pushed a branch 2.0.0-alpha back to this repo to keep track of what is published as alpha.

kumarrishav · 2021-08-16T21:29:02Z

test/test-validation.js

@@ -435,7 +429,7 @@ test('named validation', function (t) {
      t.plan(numErrorDetails + 1);
      t.ok(error, 'error.');
      error.details.forEach(function (detail) {
-        t.ok(detail.path === parameterName, 'Expected error.details.path to equal ' + parameterName);


was it failing earlier? what changed?

kumarrishav · 2021-08-16T21:31:11Z

looks good to me.

maxmil7 · 2021-08-16T21:51:04Z

👍

sumeetkakkar · 2021-08-16T21:17:49Z

.travis.yml

-  - "4"
-  - "6"
-  - "8"
+  - "12"


Maybe we should add 14

sumeetkakkar · 2021-08-16T21:20:50Z

lib/readhandlers.js

-
-    if (thing.isString(dir)) {
-        assert.ok(fs.existsSync(dir), 'Specified or default \'handlers\' directory does not exist.');
+  let handlers; let


Wondering why not let handlers, obj;?

sumeetkakkar · 2021-08-16T21:33:45Z

CHANGELOG.md

@@ -1,3 +1,9 @@
+### 1.0.12
+
+- Update to node v10, add `package-lock.json` file


+1 on Node 12. Maybe it can be changed to node 10+.
Curious, is package-lock.json for unit tests? Shouldn't unit tests always run on the latest dependency versions?

sumeetkakkar · 2021-08-16T21:51:02Z

lib/validator.js

-        makeAll: function (validators, route) {
-            var self = this;
+    makeAll(validators, route) {
+      const self = this;


I guess this is not needed now, as we are using arrow function.

sumeetkakkar · 2021-08-16T21:58:17Z

lib/validator.js

+              detail.path = [parameter.name];
+            });
+            utils.debuglog('%s', result.error.message);
+            return callback(result.error);


Since this function is using callback, should we avoid potentially returning a value in validate function?
i.e. change this to
callback(result.error); return;

sumeetkakkar · 2021-08-16T21:58:47Z

lib/validator.js

+            return callback(result.error);
+          }
+
+          return callback(null, result.value);


Since this function is using callback, should we avoid potentially returning a value in validate function?
i.e. change this to
callback(null, result.value);

sumeetkakkar · 2021-08-16T22:00:32Z

lib/validator.js

@@ -149,130 +140,134 @@ module.exports = function validator(options) {
 * @returns {*}
 */
 function refresolver(schemas, value) {
-    var id, refschema, path, fragment, paths;
+  let id; let refschema; let path; let fragment; let


This IMO is weird from readability. Should we instead define all as one let or define them in separate lines?

sumeetkakkar · 2021-08-16T22:22:22Z

Few minor comments. Otherwise looks good for semver major.

ESRogs · 2021-09-26T08:18:40Z

Is anyone able to merge this? Would be nice to get a resolution to: #94

López, Diego added 9 commits March 4, 2021 15:15

[update-enjoi] Update enjoi version, add joi module, and add the pack…

b0fc3c2

…age-lock.json file

[update-enjoi] Update enjoi implementation, add schema extension for …

123a122

…file types

[update-enjoi] Update a test case using stripUnknown option, update h…

4e85445

…ow a test case catch a path parameter

[update-lint] Add eslint as lint tool instead of jshint

44c4127

[update-lint] fix lint problems

748f3a7

[update-coverage] Use nyc instead of Istambul for code coverage

53cc3ed

[master] Add node 10 and 12 to travis file, update tape to latest ver…

bc2906a

…sion

[master] Add changes to CHANGELOG.md file, update version in package.…

3311e2e

…json file

[master] Update package-lock.json file

ff109fa

rtrys reviewed Mar 8, 2021

View reviewed changes

tikochato reviewed Mar 8, 2021

View reviewed changes

López, Diego added 3 commits March 9, 2021 10:14

[master] Sync package.json and package-lock.json for version

13ce2b7

[master] Change code to avoid lint global exceptions and unnecessary …

80d68ad

…rules

[master] Update configuration to run in node v12 or upper versions

98e2b91

RobertTheBlair suggested changes Mar 10, 2021

View reviewed changes

López, Diego added 2 commits March 15, 2021 11:22

[master] Change version to a major version

cb8202b

[master] Update package version to alpha version

35a9fc4

dlobregon mentioned this pull request Mar 18, 2021

Update "swaggerize-routes" to alpha version. krakenjs/swaggerize-express#142

Open

dlobregon mentioned this pull request Mar 26, 2021

Add swaggerize-routes alpha version. krakenjs/swaggerize-express#143

Open

kumarrishav reviewed Aug 16, 2021

View reviewed changes

sumeetkakkar reviewed Aug 16, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update for enjoi and tape packages, change dev dependencies. #97

Update for enjoi and tape packages, change dev dependencies. #97

dlobregon commented Mar 8, 2021

rtrys Mar 8, 2021

dlobregon Mar 9, 2021

rtrys Mar 8, 2021

rtrys Mar 8, 2021

dlobregon Mar 9, 2021

rtrys Mar 8, 2021

rtrys Mar 8, 2021

dlobregon Mar 9, 2021

tikochato Mar 8, 2021

dlobregon Mar 9, 2021

RobertTheBlair left a comment •

edited

Loading

RobertTheBlair Mar 10, 2021

sumeetkakkar Aug 16, 2021

RobertTheBlair Mar 10, 2021

grawk commented Mar 16, 2021

kumarrishav Aug 16, 2021

kumarrishav commented Aug 16, 2021

maxmil7 commented Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar Aug 16, 2021

sumeetkakkar commented Aug 16, 2021

ESRogs commented Sep 26, 2021 •

edited

Loading

	for (let i = 1; i < paths.length && fragment; i++) {
	for (let i = 1; i < paths.length && fragment; i+=1) {

		@@ -1,3 +1,9 @@
		### 1.0.12

		- Update to node v10, add `package-lock.json` file

Update for enjoi and tape packages, change dev dependencies. #97

Are you sure you want to change the base?

Update for enjoi and tape packages, change dev dependencies. #97

Conversation

dlobregon commented Mar 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobertTheBlair left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grawk commented Mar 16, 2021

Choose a reason for hiding this comment

kumarrishav commented Aug 16, 2021

maxmil7 commented Aug 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sumeetkakkar commented Aug 16, 2021

ESRogs commented Sep 26, 2021 • edited Loading

RobertTheBlair left a comment •

edited

Loading

ESRogs commented Sep 26, 2021 •

edited

Loading