Skip to content

Conversation

@paullinator
Copy link
Member

@paullinator paullinator commented Jun 19, 2024

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Description

New branch to address comments from #322


@paullinator paullinator force-pushed the paul/addRangoExchange branch 2 times, most recently from 08408c4 to 601e5df Compare June 20, 2024 17:56
@paullinator paullinator mentioned this pull request Jun 23, 2024
2 tasks
@paullinator
Copy link
Member Author

My new commits address comments from original PR #322


metaRequest
.then(metaResponse => {
metaRequestPromise = metaRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be broken out as a function and calling it should be governed by a global boolean instead of the promise. Otherwise, a rejected means the user will never use this plugin for the rest of their session.

return metaResponse.json()
return await metaResponse.json()
} else {
return await metaResponse.text()
Copy link
Contributor

Choose a reason for hiding this comment

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

Throw the return from .text() so the log is more useful than a cleaner error

disableEstimate: true,
slippage: DEFAULT_SLIPPAGE,
...(referrerAddress !== undefined &&
...(referrerAddress != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: Can we just enforce that these exist where they're declared and get rid of this spread/boolchain nonsense?

Copy link
Member Author

Choose a reason for hiding this comment

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

No we shouldn't enforce they exist as the plugin should be usable without referral fees.

@paullinator paullinator force-pushed the paul/addRangoExchange branch from f5a7988 to 04a4302 Compare June 24, 2024 23:13
Some chains are disabled until we write code to support them but add them commented out anyway so we know their codes
@paullinator paullinator force-pushed the paul/addRangoExchange branch from 04a4302 to 6f7be5e Compare June 24, 2024 23:25
@paullinator paullinator merged commit 292bab9 into master Jun 24, 2024
@paullinator paullinator deleted the paul/addRangoExchange branch June 24, 2024 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants