Rename fields in taker request by feuGeneA · Pull Request #5 · 0xProject/quote-server

feuGeneA · 2020-05-14T21:44:14Z

In lieu of CI, a log of a test run on the latest commit:

14May17:56:34 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params=]$ git log -1
commit 4d16e888ae43b292dc02f91f670006217627ed41
Author: F. Eugene Aumson <feuGeneA@users.noreply.github.com>
Date:   Thu May 14 17:56:24 2020 -0400

    Add .prettierignore file
14May17:56:37 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params=]$ git status
On branch feat/rename-indicative-quote-params
Your branch is up-to-date with 'origin/feat/rename-indicative-quote-params'.
nothing to commit, working tree clean
14May17:56:39 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params=]$ yarn clean && yarn build && yarn test && yarn lint
yarn run v1.19.1
$ shx rm -rf lib
Done in 0.13s.
yarn run v1.19.1
$ tsc -b
Done in 2.33s.
yarn run v1.19.1
$ NODE_ENV=test mocha --require source-map-support/register --require make-promises-safe ./lib/test/**/**/*_test.js --exit


  api key handler
    ✓ reject when canMakerControlSettlement == false & no API key specified
    ✓ accept when canMakerControlSettlement == false & API Key specified
    ✓ accept when canMakerControlSettlement == true & no API Key specified
    ✓ accept when canMakerControlSettlement == true & API Key specified

  taker request handler
    ✓ should defer to quoter and return response for firm quote
    ✓ should succeed without an API key if `canMakerControlSettlement` is set to `true` when requesting a firm quote
    ✓ should defer to quoter and return response for indicative quote
    ✓ should handle empty indicative quote
    ✓ should handle empty firm quote
    ✓ should invalidate a bad request

  submit request handler
    ✓ should defer to quoter and return response for submit request
    ✓ should succeed without an API key when submitting a fill
    ✓ should invalidate a bad request
    ✓ should handle empty indicative quote


  14 passing (121ms)

Done in 0.62s.
yarn run v1.19.1
$ run-p tslint prettier:lint
$ tslint --format stylish --project .
$ yarn prettier --list-different
$ prettier '**/*.{ts,tsx,json,md}' --config .prettierrc --list-different
no-unused-variable is deprecated. Since TypeScript 2.9. Please use the built-in compiler checks instead.
Done in 3.50s.

Unpin upon merge and republication of 0xProject/quote-server#5

steveklebanoff · 2020-05-18T23:31:13Z

CHANGELOG.json

@@ -0,0 +1,20 @@
+[
+    {
+        "version": "0.1.0",


sup w/ these duplicate entries?

Thanks, addressed in 598b8f9

steveklebanoff

Two minor comments but otherwise LGTM -- nice work

steveklebanoff · 2020-05-18T23:33:22Z

package.json

-        "prettier:ci": "prettier --list-different '**/*.{ts,tsx,json,md}' --config .prettierrc",
+        "tslint": "tslint --format stylish --project .",
+        "tslint:fix": "yarn tslint --fix",
+        "prettier": "prettier '**/*.{ts,tsx,json,md}' --config .prettierrc",


I know we've had this convo before, but I personally would like to keep our script patterns the same as the monorepo.

If you feel strongly about grouping prettier & tslint into one "lint" command I see your point, but I do think it is a bit strange to have different patterns between this repo & the monorepo

I appreciate your effort to keep us consistent. FYI I'm on a crusade to do this everywhere now. Every time I work in a monorepo package I'm updating it to include prettier in lint. Though, so far I've only done it to asset-swapper and migrations :)

I forgot to update the version number and changes in a recent commit.

steveklebanoff · 2020-05-21T16:28:25Z

CHANGELOG.json

@@ -0,0 +1,29 @@
+[


Does this CHANGELOG.json actually do anything? I was expecting a publish entry in scripts to investigate, but I don't see one 🤔

Nope, it's just me keeping a manual record. Having it actually do something would be great, but I figured doing things manually would be better than doing nothing at all.

steveklebanoff

I'm curious about CHANGELOG.json and the absence of a publish script but LGTM regardless

21May12:46:46 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params>]$ git log -1 commit 6f94cfd1b9f8f50fdb61ead4f498f567dd268b8e Author: F. Eugene Aumson <feuGeneA@users.noreply.github.com> Date: Thu May 21 12:45:31 2020 -0400 Migrate CHANGELOG from .json to .md 21May12:46:47 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params>]$ git status On branch feat/rename-indicative-quote-params Your branch is ahead of 'origin/feat/rename-indicative-quote-params' by 1 commit. (use "git push" to publish your local commits) nothing to commit, working tree clean 21May12:46:53 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params>]$ yarn lint yarn run v1.19.1 $ run-p tslint prettier:lint $ tslint --format stylish --project . $ yarn prettier --list-different $ prettier '**/*.{ts,tsx,json,md}' --config .prettierrc --list-different no-unused-variable is deprecated. Since TypeScript 2.9. Please use the built-in compiler checks instead. Done in 3.12s.

That is, from devDependencies to regular dependencies. This is needed because we export an interface (serverRoutes()) that depends on those types. Without this change, the monorepo's `test-publish`, running with an asset-swapper modified to depend on this package, gave: node_modules/@0x/quote-server/lib/src/router.d.ts:2:70 - error TS2307: Cannot find module 'express-serve-static-core'. 2 export declare const serverRoutes: (quoteStrategy: Quoter) => import("express-serve-static-core").Router; ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ("express-serve-static-core" is a direct dependency of @types/express.)

Clean test run: 28May18:01:47 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params>]$ git status On branch feat/rename-indicative-quote-params Your branch is ahead of 'origin/feat/rename-indicative-quote-params' by 1 commit. (use "git push" to publish your local commits) nothing to commit, working tree clean 28May18:01:49 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params>]$ git log -1 commit 6bce4da342c3369c67e990d583b2add8a04a9978 Author: F. Eugene Aumson <feuGeneA@users.noreply.github.com> Date: Wed May 27 18:33:18 2020 -0400 Add distinctive types for RFQM requests 28May18:01:53 [2 jobs] ~/dev/quote-server[feat/rename-indicative-quote-params>]$ btl yarn run v1.19.1 $ tsc -b Done in 0.35s. yarn run v1.19.1 $ NODE_ENV=test mocha --require source-map-support/register --require make-promises-safe ./lib/test/**/**/*_test.js --exit api key handler ✓ reject when canMakerControlSettlement == false & no API key specified ✓ accept when canMakerControlSettlement == false & API Key specified ✓ accept when canMakerControlSettlement == true & no API Key specified ✓ accept when canMakerControlSettlement == true & API Key specified taker request handler ✓ should defer to quoter and return response for firm quote (41ms) ✓ should succeed without an API key if `canMakerControlSettlement` is set to `true` when requesting a firm quote ✓ should defer to quoter and return response for indicative quote ✓ should handle empty indicative quote ✓ should handle empty firm quote ✓ should invalidate a bad request submit request handler ✓ should defer to quoter and return response for submit request ✓ should succeed without an API key when submitting a fill ✓ should invalidate a bad request ✓ should handle empty indicative quote 14 passing (166ms) Done in 0.77s. yarn run v1.19.1 $ run-p tslint prettier:lint $ yarn prettier --list-different $ tslint --format stylish --project . $ prettier '**/*.{ts,tsx,json,md}' --config .prettierrc --list-different no-unused-variable is deprecated. Since TypeScript 2.9. Please use the built-in compiler checks instead. Done in 4.35s.

steveklebanoff · 2020-06-08T21:20:52Z

src/types.ts

+    quoteExpiry: number;
+}
+
+export type FirmQuote = RFQTFirmQuote | RFQMFirmQuote;


Lookin' good 👍

feuGeneA added 4 commits May 14, 2020 17:34

Rename fields in taker indicative quote request

7da1eec

Encapsulate prettier within yarn lint & ...fix

6e3cecf

Add and update CHANGELOG.json

c90569c

Add .prettierignore file

4d16e88

feuGeneA added a commit to 0xProject/0x-monorepo that referenced this pull request May 15, 2020

Pin @0x/quote-server to "next" tag

9e864df

Unpin upon merge and republication of 0xProject/quote-server#5

feuGeneA changed the title ~~Rename fields in taker indicative quote request~~ Rename fields in taker request May 15, 2020

feuGeneA mentioned this pull request May 15, 2020

asset-swapper: Use @0x/quote-server, not local typedefs; and adapt to renamed taker request parameters 0xProject/0x-monorepo#2582

Merged

2 tasks

steveklebanoff reviewed May 18, 2020

View reviewed changes

steveklebanoff suggested changes May 18, 2020

View reviewed changes

feuGeneA added 2 commits May 20, 2020 12:50

Fix mistake in CHANGELOG.json

598b8f9

I forgot to update the version number and changes in a recent commit.

Add missing exports SubmitRequest and SubmitReceipt

23d0ef3

feuGeneA requested a review from steveklebanoff May 20, 2020 22:26

steveklebanoff reviewed May 21, 2020

View reviewed changes

steveklebanoff approved these changes May 21, 2020

View reviewed changes

feuGeneA added 3 commits May 21, 2020 12:47

Post-publish version bump

d377c47

feuGeneA force-pushed the feat/rename-indicative-quote-params branch from 81f968d to ccf3819 Compare May 28, 2020 22:10

feuGeneA force-pushed the feat/rename-indicative-quote-params branch from ccf3819 to 37f5d6c Compare May 28, 2020 22:18

Export RFQT- and RFQM-specific request types

7671f10

steveklebanoff reviewed Jun 8, 2020

View reviewed changes

steveklebanoff approved these changes Jun 8, 2020

View reviewed changes

feuGeneA mentioned this pull request Jun 16, 2020

feat: support renamed parameters in RFQT maker endpoint 0xProject/0x-api#258

Merged

1 task

feuGeneA merged commit 09346d3 into master Jun 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename fields in taker request#5

Rename fields in taker request#5
feuGeneA merged 11 commits intomasterfrom
feat/rename-indicative-quote-params

feuGeneA commented May 14, 2020 •

edited

Loading

Uh oh!

steveklebanoff May 18, 2020

Uh oh!

feuGeneA May 20, 2020

Uh oh!

steveklebanoff left a comment

Uh oh!

steveklebanoff May 18, 2020

Uh oh!

feuGeneA May 20, 2020 •

edited

Loading

Uh oh!

steveklebanoff May 21, 2020

Uh oh!

steveklebanoff May 21, 2020

Uh oh!

feuGeneA May 21, 2020

Uh oh!

steveklebanoff left a comment

Uh oh!

steveklebanoff Jun 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

feuGeneA commented May 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steveklebanoff May 18, 2020

Choose a reason for hiding this comment

Uh oh!

feuGeneA May 20, 2020

Choose a reason for hiding this comment

Uh oh!

steveklebanoff left a comment

Choose a reason for hiding this comment

Uh oh!

steveklebanoff May 18, 2020

Choose a reason for hiding this comment

Uh oh!

feuGeneA May 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steveklebanoff May 21, 2020

Choose a reason for hiding this comment

Uh oh!

steveklebanoff May 21, 2020

Choose a reason for hiding this comment

Uh oh!

feuGeneA May 21, 2020

Choose a reason for hiding this comment

Uh oh!

steveklebanoff left a comment

Choose a reason for hiding this comment

Uh oh!

steveklebanoff Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feuGeneA commented May 14, 2020 •

edited

Loading

feuGeneA May 20, 2020 •

edited

Loading