Skip to content

Fix/replace restler - #1

Open
adeleKayyyy wants to merge 16 commits into
masterfrom
fix/replace-restler
Open

Fix/replace restler#1
adeleKayyyy wants to merge 16 commits into
masterfrom
fix/replace-restler

Conversation

@adeleKayyyy

Copy link
Copy Markdown
  • replaced restler with fetch
  • added some functions to reduce duplication

@chrishutchinson

Copy link
Copy Markdown

In general this looks good, a few overall points:

  • We'll need to get a solid test suite running to be really confident we haven't broken anything (see my note in Added jest framework #2)
  • There's lots of var and other ES5 stuff in this codebase (purely because of how old it is). We could probably consider upgrading this to use let, const, arrow functions, and other good ES6 stuff which has been supported since Node 6 or 8 in most cases. More details of support for specific features: https://node.green/
  • It's quite a long file, lots of stuff going on. I wonder if we could modularise it a little? Not sure exactly the best way to do that, it'd be good to get your thoughts on that

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.

2 participants