You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hey @jwerre this code was written by me when I added these tests. The thing is, that the RegEx that is about to be tested allows nearly the whole unicode set and I do lots of permutations in order to cover them. This is course takes lot of time.
From my pov we should not remove them but we could update the docs and describe why this test is so slow (and give some background info).
We can also add an instant remedy by describing how to use regex pattern to filter out mocha tests that are not of focus right now: https://mochajs.org/#-grep-regexp-g-regexp
@jankapunkt Yes I remember when you wrote these test. Nice work by the way—I agree with you that they were needed. I guess I'm not clear why you wouldn't want to add is.js as a dependance under the same organization. It's not really oauth specific, it's not likely to change, and it would shave 5.5 sec off the unit tests. It just doesn't make sense to run these tests with every deployment of node-oauth-server when they can be run independently whenever there are changes to is.js.
I put is.js into it's own repo. @HappyZombies would you mind publishing this to NPM? Once that's done I'll update the code and create a PR. @jankapunkt feel free to clone and make any changes you see fit.
I am sorry to be nagging on the wording, but I think the name is wrong. it is not a "is-unicode" package because some of the validators are not unicode but simple ascii. It is actually something like oauth2-formats (naming similar to something like ajv-formats).
Activity
jankapunkt commentedon Nov 11, 2021
Hey @jwerre this code was written by me when I added these tests. The thing is, that the RegEx that is about to be tested allows nearly the whole unicode set and I do lots of permutations in order to cover them. This is course takes lot of time.
From my pov we should not remove them but we could update the docs and describe why this test is so slow (and give some background info).
We can also add an instant remedy by describing how to use regex pattern to filter out mocha tests that are not of focus right now: https://mochajs.org/#-grep-regexp-g-regexp
What do you think?
jwerre commentedon Nov 11, 2021
@jankapunkt Yes I remember when you wrote these test. Nice work by the way—I agree with you that they were needed. I guess I'm not clear why you wouldn't want to add
is.jsas a dependance under the same organization. It's not really oauth specific, it's not likely to change, and it would shave 5.5 sec off the unit tests. It just doesn't make sense to run these tests with every deployment ofnode-oauth-serverwhen they can be run independently whenever there are changes to is.js.jankapunkt commentedon Nov 11, 2021
Ah now I get, you want to add them as internal dependency. We could do this.
jankapunkt commentedon Nov 13, 2021
@jwerre @HappyZombies I would start this as a new repo under this org if you both agree
jwerre commentedon Nov 21, 2021
I put
is.jsinto it's own repo. @HappyZombies would you mind publishing this to NPM? Once that's done I'll update the code and create a PR. @jankapunkt feel free to clone and make any changes you see fit.jankapunkt commentedon Nov 22, 2021
@jwerre @HappyZombies I would first make some additions, like adding the namespace, updating the README, link the RFC docs etc. I'll link the PR here
jankapunkt commentedon Nov 22, 2021
See node-oauth/formats#1
Uzlopak commentedon Dec 11, 2021
I am sorry to be nagging on the wording, but I think the name is wrong. it is not a "is-unicode" package because some of the validators are not unicode but simple ascii. It is actually something like oauth2-formats (naming similar to something like ajv-formats).
jankapunkt commentedon Dec 11, 2021
@jwerre what do you think about renaming? It's at least not published, yet.
jwerre commentedon Dec 11, 2021
Yes! Good call @Uzlopak
jankapunkt commentedon Dec 11, 2021
I will rename and then publish, so we can finally integrate it :-)
jankapunkt commentedon Dec 11, 2021
Published 1.0.0: https://www.npmjs.com/package/@node-oauth/formats
Let's keep this open until we integrated it into the main package
feature(core): extract is.js into standalone package @node-oauth/form…
jorenvandeweyer commentedon Dec 19, 2021
Resolved by #108