Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename {Rfqt=>Rfq} for types in Asset Swapper #179

Merged
merged 6 commits into from
Mar 23, 2021

Conversation

phil-ociraptor
Copy link
Contributor

Description

Super tiny change to make Rfqt=>Rfq for types in Asset Swapper. This is because these types will be reused for RFQM

Testing instructions

Types of changes

  • Refactor

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

nativeExclusivelyRFQT?: boolean;
altRfqtAssetOfferings?: AltRfqtMakerAssetOfferings;
nativeExclusivelyRFQ?: boolean;
altRfqtAssetOfferings?: AltRfqMakerAssetOfferings;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

left this field to be called altRfqtAssetOfferings because not sure if this will be separate for RFQT and RFQM

Copy link
Contributor

@alexkroeger alexkroeger Mar 19, 2021

Choose a reason for hiding this comment

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

I think we should rename it if we're making this type RFQ-T or M agnostic

}

/**
* gasPrice: gas price to determine protocolFee amount, default to ethGasStation fast amount
*/
export interface SwapQuoteRequestOpts extends GetMarketOrdersOpts {
gasPrice?: BigNumber;
rfqt?: RfqtRequestOpts;
rfqt?: RfqRequestOpts;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

also left this field to be called rfqt because I imagine there might be a separate field for rfqm but not sure

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 left rfqt bc RFQM will be separate from swapQuoter

@@ -119,7 +119,7 @@ export async function returnQuoteFromAltMMAsync<ResponseT>(
makerToken: string,
takerToken: string,
maxResponseTimeMs: number,
altRfqtAssetOfferings: AltRfqtMakerAssetOfferings,
altRfqtAssetOfferings: AltRfqMakerAssetOfferings,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this field might need to be renamed?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea best to rename this as well

@phil-ociraptor phil-ociraptor merged commit 06b3464 into development Mar 23, 2021
@phil-ociraptor phil-ociraptor deleted the phil/rename-rfqt branch March 23, 2021 22:21
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