feat(Utility): expose UUID generator #303

benjamincharity · 2019-05-24T13:04:23Z

ISSUES CLOSED: #91

Expose UUID generator

codecov · 2019-05-24T13:28:54Z

Codecov Report

Merging #303 into release will increase coverage by 0.13%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           release     #303      +/-   ##
===========================================
+ Coverage    80.53%   80.67%   +0.13%     
===========================================
  Files           94       95       +1     
  Lines         1115     1123       +8     
  Branches       186      186              
===========================================
+ Hits           898      906       +8     
  Misses         217      217

Flag	Coverage Δ
#coercion	`100% <ø> (ø)`	⬆️
#keycodes	`100% <ø> (ø)`	⬆️
#regex	`100% <ø> (ø)`	⬆️
#testing	`33.82% <ø> (-0.25%)`	⬇️
#utilities	`95.15% <100%> (+0.05%)`	⬆️

Impacted Files	Coverage Δ
ngx-tools/testing/src/utilities/event-objects.ts	`100% <ø> (ø)`	⬆️
ngx-tools/src/uuid/uuid.ts	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f323396...7b198da. Read the comment docs.

codecov · 2019-05-24T13:28:54Z

Codecov Report

Merging #303 into release will increase coverage by 0.13%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           release     #303      +/-   ##
===========================================
+ Coverage    80.53%   80.67%   +0.13%     
===========================================
  Files           94       95       +1     
  Lines         1115     1123       +8     
  Branches       186      186              
===========================================
+ Hits           898      906       +8     
  Misses         217      217

Flag	Coverage Δ
#coercion	`100% <ø> (ø)`	⬆️
#keycodes	`100% <ø> (ø)`	⬆️
#regex	`100% <ø> (ø)`	⬆️
#testing	`33.82% <ø> (-0.25%)`	⬇️
#utilities	`95.15% <100%> (+0.05%)`	⬆️

Impacted Files	Coverage Δ
ngx-tools/testing/src/utilities/event-objects.ts	`100% <ø> (ø)`	⬆️
ngx-tools/src/uuid/uuid.ts	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e49ecb...3298a8a. Read the comment docs.

shani-terminus · 2019-05-28T11:48:01Z

ngx-tools/src/jwt-token-managment/effects.spec.ts

@@ -96,6 +96,7 @@ describe(`JWT Token Effects`, function() {
    };

    test(`should provide a cookie and store cookie message if the cookie is set`, () => {
+      // tslint:disable-next-line deprecation


What is getting deprecated, here, 118, 136, and 155?

shani-terminus · 2019-05-28T11:55:22Z

ngx-tools/src/retry-with-backoff/retry-with-backoff.spec.ts

@@ -14,6 +14,7 @@ describe(`retryWithBackoff`, function() {
    const error = new Error('bar');
    const seenValues: {[idx: number]: number} = {};

+    // tslint:disable-next-line deprecation


Okay, so I will assume of is getting deprecated.
Do we have a plan of updating our code?

We're actually just getting around a TSLint bug. The deprecation rule is triggered even though of itself is not deprecated; only certain overloads are. We did this a bit here also GetTerminus/terminus-ui#1488

There is an open PR to fix this which would allow us to remove all of these disables. But since TSLint itself is deprecated I'm not holding my breath on it getting fixed.

shani-terminus · 2019-05-28T12:17:42Z

ngx-tools/testing/src/utilities/event-objects.spec.ts

@@ -50,7 +50,7 @@ describe(`event-objects`, function() {
      const actual: KeyboardEvent = createKeyboardEvent('keydown', KEYS.ENTER, target);
      expect(actual.code).toEqual(KEYS.ENTER.code);
      expect(actual.key).toEqual(KEYS.ENTER.code);
-      expect(actual.keyCode).toEqual(KEYS.ENTER.keyCode);
+      expect(actual.code).toEqual(KEYS.ENTER.code);


duplicate of line 51
have we removed all references to keyCode?

Removed.

I didn't do a full search as that's not the basis of this PR. I'm honestly not entirely sure how these were missed in the linting PR. They should have been flagged by the deprecation rule when it was implemented.

shani-terminus · 2019-05-28T12:24:27Z

ngx-tools/src/uuid/uuid.spec.ts

+
+      // Test Validity
+      if (!uuidRegex.test(newUuid)) {
+        done.fail(new Error('This is the error'));


I know it's only a test but the message could be more descriptive: 'UUID is not valid'

Whoops.. yeah definitely should make that better.

shani-terminus · 2019-05-28T12:34:21Z

Looks like public-api.ts still needs to be updated.

ISSUES CLOSED: #91

benjamincharity · 2019-05-28T12:48:08Z

Huh not sure how that got dropped. Re-added to public api 👍

atlwendy · 2019-05-28T15:32:57Z

package.json

@@ -220,7 +220,8 @@
    "tsickle": "^0.34.0",
    "tslint": "^5.16.0",
    "typescript": "~3.2.1",
-    "validate-commit-msg": "^2.14.0"
+    "validate-commit-msg": "^2.14.0",
+    "window-crypto": "^1.1.0"


Do you need this to use window.crypto.getRandomValues?

Only for testing. Jest doesn't have a polyfill for window.crypto by default. So in order to test it as accurately as possible, I'm just polyfilling the functionality as a whole (vs just mocking it).

atlwendy · 2019-05-28T15:34:09Z

tools/jest-global-mocks.ts

    setItem: (key: string, value: any) => {
      storage[key] = value || '';
      return storage[key];
    },
    removeItem: (key: string) => delete storage[key],
-    clear: () => storage = {},
+    clear: () => (storage = {}),


Are the parenthesis needed or for better readability?

Kind of. Creating an assignment in a return statement can be confusing so we have a lint rule warning about this.

The () is just us being explicit that something is happening inside, then it returns.

A better way for me to phrase it: It is not more readable, but the intent should be more clear.

benjamincharity requested review from atlwendy and shani-terminus as code owners May 24, 2019 13:04

benjamincharity self-assigned this May 24, 2019

benjamincharity mentioned this pull request May 24, 2019

91 UUID v2 #302

Closed

benjamincharity force-pushed the 91-uuid-clean-branch branch from d99f9aa to 7b198da Compare May 24, 2019 13:24

shani-terminus suggested changes May 28, 2019

View reviewed changes

benjamincharity force-pushed the 91-uuid-clean-branch branch from 7b198da to 705e5bf Compare May 28, 2019 12:37

benjamincharity requested a review from shani-terminus May 28, 2019 12:39

benjamincharity force-pushed the 91-uuid-clean-branch branch from 705e5bf to 957c842 Compare May 28, 2019 12:41

benjamincharity added 2 commits May 28, 2019 08:47

feat(Utility): expose UUID generator

cfb5c36

ISSUES CLOSED: #91

style: fix lint violations

3298a8a

benjamincharity force-pushed the 91-uuid-clean-branch branch from 957c842 to 3298a8a Compare May 28, 2019 12:47

shani-terminus approved these changes May 28, 2019

View reviewed changes

atlwendy approved these changes May 28, 2019

View reviewed changes

benjamincharity merged commit beee1f7 into release May 28, 2019

benjamincharity deleted the 91-uuid-clean-branch branch June 12, 2019 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Utility): expose UUID generator #303

feat(Utility): expose UUID generator #303

benjamincharity commented May 24, 2019

codecov bot commented May 24, 2019

codecov bot commented May 24, 2019 •

edited

Loading

shani-terminus May 28, 2019

shani-terminus May 28, 2019

benjamincharity May 28, 2019

shani-terminus May 28, 2019

benjamincharity May 28, 2019

shani-terminus May 28, 2019

benjamincharity May 28, 2019

shani-terminus commented May 28, 2019

benjamincharity commented May 28, 2019

atlwendy May 28, 2019

benjamincharity May 28, 2019

atlwendy May 28, 2019

benjamincharity May 28, 2019

benjamincharity May 28, 2019

feat(Utility): expose UUID generator #303

feat(Utility): expose UUID generator #303

Conversation

benjamincharity commented May 24, 2019

codecov bot commented May 24, 2019

Codecov Report

codecov bot commented May 24, 2019 • edited Loading

Codecov Report

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

shani-terminus commented May 28, 2019

benjamincharity commented May 28, 2019

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

codecov bot commented May 24, 2019 •

edited

Loading