[event-hubs] fix eslint errors (round 1) #13013

chradek · 2020-12-23T19:27:02Z

Round 1 addressing #10777
These changes addresses ~300 of the eslint errors and some warnings found by running eslint with the TSDoc rules disabled.

This leaves 13 errors and 51 warnings left after this PR is merged.

5 'options parameter type is not prefixed with the class|method name' errors. Fixing this changes the public API so holding off on this.
4 'Promise executor functions should not be async' errors. Fixing this will take a lot of care and I would want these to be reviewed on their own so they don't get lost in the noise.
2 'X was used before it was defined' errors. These are somewhat misleading because they are used within methods and defined before those methods can be invoked. We might be able to fix these if we refactor the async promise executor functions.
1 'Argument X should be types with a non-any type'. This isn't an external API so this should be fine. Need to think about best way to disable this (maybe just eslint-disable-line).
1 'N is already defined'. This is due to a constant matching an interface name.

sdk/eventhub/event-hubs/src/eventData.ts

HarshaNalluru · 2020-12-23T21:06:56Z

sdk/eventhub/event-hubs/src/util/parseEndpoint.ts

@@ -8,7 +8,7 @@
 * @internal
 */
 export function parseEndpoint(endpoint: string): { host: string; hostname: string; port?: string } {
-  const hostMatch = endpoint.match(/.*:\/\/([^\/]*)/i);
+  const hostMatch = endpoint.match(/.*:\/\/([^/]*)/i);


Hmm.. doesn't make a difference, right?

Right. ESLint was complaining because that backslash was unnecessary.

sdk/eventhub/event-hubs/test/sender.spec.ts

deyaaeldeen

I am super happy with the push to fix linting issues. One thing I left a comment about is that we should move away from any as an argument type and use unknown instead.

deyaaeldeen · 2020-12-28T16:14:47Z

sdk/eventhub/event-hubs/src/dataTransformer.ts

@@ -24,14 +24,15 @@ export const defaultDataTransformer = {
   * - multiple: true | undefined.
   */
  encode(body: any): any {
+    // eslint-disable-line @typescript-eslint/explicit-module-boundary-types
    let result: any;


can we change any to unknown in the argument type throughout to fix the linting the issue? it could be a bit painful sometimes but it is safer to work with.

Another option is to refactor the if/else block to return early so that we don't need to have the result variable at all.

if (isBuffer(body) { return message.data_section(body); } try { const bodyStr = JSON.stringify(body); return message.data_section(Buffer.from(bodyStr, "utf8")); } catch (err) { }

@ramya-rao-a sorry for the confusion by putting my comment on line 28 instead of line 26. I meant to do this change for arguments only (body in this case). This is where eslint complains.

Removed all of the @typescript-eslint/explicit-module-boundary-types overrides in #13285

deyaaeldeen · 2020-12-28T16:15:34Z

sdk/eventhub/event-hubs/src/dataTransformer.ts

@@ -57,6 +58,7 @@ export const defaultDataTransformer = {
   * @return {*} decoded body or the given body as-is.
   */
  decode(body: any): any {
+    // eslint-disable-line @typescript-eslint/explicit-module-boundary-types
    let processedBody: any = body;


deyaaeldeen · 2020-12-28T16:15:51Z

sdk/eventhub/event-hubs/src/eventDataBatch.ts

@@ -30,6 +31,7 @@ const smallMessageMaxBytes = 255;
 * @ignore
 */
 export function isEventDataBatch(eventDataBatch: any): eventDataBatch is EventDataBatch {
+  // eslint-disable-line @typescript-eslint/explicit-module-boundary-types
  return (


deyaaeldeen · 2020-12-28T16:18:08Z

sdk/eventhub/event-hubs/src/log.ts

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.

+/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
+


deyaaeldeen · 2020-12-28T16:21:04Z

sdk/eventhub/event-hubs/src/util/error.ts

@@ -1,6 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.

+// Disable eslint rule since these functions aren't part of the public API.
+/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
+


richardpark-msft

Still reviewing, but noticed something interesting.

sdk/eventhub/event-hubs/src/eventHubReceiver.ts

chradek · 2021-01-19T23:53:51Z

Closing in favor of #13285

Round 1 addressing #10777 Replaces #13013 This leaves 5 errors and 15 warnings (excluding TSDoc) after this PR is merged. - 4 'Promise executor functions should not be async' errors. Fixing this will take a lot of care and I would want these to be reviewed on their own so they don't get lost in the noise. - 1 'N is already defined'. This is due to a constant matching an interface name.

Round 1 addressing Azure#10777 Replaces Azure#13013 This leaves 5 errors and 15 warnings (excluding TSDoc) after this PR is merged. - 4 'Promise executor functions should not be async' errors. Fixing this will take a lot of care and I would want these to be reviewed on their own so they don't get lost in the noise. - 1 'N is already defined'. This is due to a constant matching an interface name.

chradek added 2 commits December 23, 2020 10:56

[event-hubs] fix eslint errors (round 1)

8c31cc3

[event-hubs] run formatter

8b92d66

chradek requested review from HarshaNalluru, ramya-rao-a and richardpark-msft as code owners December 23, 2020 19:27

ghost added the Event Hubs label Dec 23, 2020

HarshaNalluru reviewed Dec 23, 2020

View reviewed changes

sdk/eventhub/event-hubs/src/eventData.ts Outdated Show resolved Hide resolved

chradek added 2 commits December 23, 2020 12:55

[event-hubs] use isDefined more

2788f59

[event-hubs] use isDefined more

560714a

HarshaNalluru reviewed Dec 23, 2020

View reviewed changes

sdk/eventhub/event-hubs/test/sender.spec.ts Show resolved Hide resolved

HarshaNalluru approved these changes Dec 23, 2020

View reviewed changes

deyaaeldeen reviewed Dec 28, 2020

View reviewed changes

richardpark-msft reviewed Jan 5, 2021

View reviewed changes

sdk/eventhub/event-hubs/src/eventHubReceiver.ts Show resolved Hide resolved

chradek mentioned this pull request Jan 19, 2021

[event-hubs] fix eslint errors (round 1) #13285

Merged

chradek closed this Jan 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[event-hubs] fix eslint errors (round 1) #13013

[event-hubs] fix eslint errors (round 1) #13013

chradek commented Dec 23, 2020

HarshaNalluru Dec 23, 2020

chradek Jan 5, 2021

deyaaeldeen left a comment

deyaaeldeen Dec 28, 2020

ramya-rao-a Jan 4, 2021

deyaaeldeen Jan 5, 2021 •

edited

Loading

chradek Jan 19, 2021

deyaaeldeen Dec 28, 2020

deyaaeldeen Dec 28, 2020

deyaaeldeen Dec 28, 2020

deyaaeldeen Dec 28, 2020

richardpark-msft left a comment

chradek commented Jan 19, 2021

[event-hubs] fix eslint errors (round 1) #13013

[event-hubs] fix eslint errors (round 1) #13013

Conversation

chradek commented Dec 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deyaaeldeen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deyaaeldeen Jan 5, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richardpark-msft left a comment

Choose a reason for hiding this comment

chradek commented Jan 19, 2021

deyaaeldeen Jan 5, 2021 •

edited

Loading