Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

WIP: Update fungible-faucet to work with agoric-sdk community-dev #59

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ivanlei
Copy link

@ivanlei ivanlei commented Apr 6, 2023

Summary

Update fungible-faucet to work with agoric-sdk community-dev

Details

Testing

@@ -1,5 +1,4 @@
// @ts-check
/* globals window, WebSocket */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like window at least is still used as a free variable here. Our tools are supposed to at least warn on such variables without this "globals" declaration. This is directly related to the outstanding bug on too much ambient authority.

Do our tools not warn if you get rid of this "globals" directive? That itself is bug. Please file an issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attn @dckc , our guardian against ambient authority ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that tool is attached to this repo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we ensure greater tool uniformity among our repos?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know any short path to get there.

And I think we've drifted pretty far off topic from this PR...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I think we've drifted pretty far off topic from this PR...

Agreed!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants