Ecosyste.ms: Timeline
Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.
julianna-ciq created a review comment on a pull request on finos/FDC3
Oh well. We can leave it, then.
julianna-ciq created a review comment on a pull request on finos/FDC3
Looks like another merge issue
julianna-ciq created a review comment on a pull request on finos/FDC3
Could this look like `const event: HeartbeatEvent = { ... }; sc.post(event);`?
julianna-ciq created a review comment on a pull request on finos/FDC3
Should we keep this commented-out line?
julianna-ciq created a review comment on a pull request on finos/FDC3
Should we leave this `console.log` line? If so, can it be a `console.debug`, maybe with a more informative message? Like `console.debug('Channel Selector - DragMouseDown starting')`
julianna-ciq created a review comment on a pull request on finos/FDC3
Should we just delete these lines?
julianna-ciq created a review comment on a pull request on finos/FDC3
Should the commented-out argument be deleted?
julianna-ciq created a review comment on a pull request on finos/FDC3
Should we just delete this line?
julianna-ciq created a comment on a pull request on finos/FDC3
Doesn’t that mean there’s no tests for returning null? I think it might be better to adjust the function to return null if the id is something like “does-not-exist” or equivalent _________________...
julianna-ciq created a review comment on a pull request on finos/FDC3
Do these commented-out lines need to stay?
julianna-ciq created a review comment on a pull request on finos/FDC3
This method only returns type `HTMLElement`, and would never return `null`. Should we continue to have `| null` in the type definition if null can't ever be returned?
julianna-ciq created a review comment on a pull request on finos/FDC3
Instead of commenting out the argument, should it just be removed?
julianna-ciq created a review comment on a pull request on finos/FDC3
It would be better if MockIFrame and MockElement were of type HTMLElement, so that you don't need to have `as unknown as HTMLElement` all over the place. Like, you might have to do it in the MockIF...
julianna-ciq created a review comment on a pull request on finos/FDC3
Is it possible to do something like this: ```ts const event: Event = { type: "message", data: msg, origin: CHANNEL_SELECTOR_URL, source, ports: [connection.port1] }; parent.dis...
julianna-ciq created a review comment on a pull request on finos/FDC3
Just flagging this line because `this` scoping is a common pain point in javascript. I think `this` refers to the promise at this point, but could refer to the class... Either way, please double...
julianna-ciq created a review comment on a pull request on finos/FDC3
See comments on ./RegisterListeners.ts
julianna-ciq created a review comment on a pull request on finos/FDC3
Is "FluffyInstrument" a real thing?
julianna-ciq created a review comment on a pull request on finos/FDC3
Why the `as requests` here? Does `AppRequestMessage` not overlap with the list of possible listener requests?
julianna-ciq created a review comment on a pull request on finos/FDC3
Would you mind pascal casing your types? It'd be consistent with the other types that have been defined. like `type SupportedListenerRequests = ...`
julianna-ciq created a review comment on a pull request on finos/FDC3
Flagging this section, since it looks like it's still a WIP