Skip to content

Conversation

@davidcostadev
Copy link
Contributor

  • Add integration test to ensure that the two functions will work together and pass after it is refactored.
  • Move handle fetch stuff to API file.

@lucianoratamero
Copy link
Contributor

@davidcostadev , as always, thanks for your hard work!

I'm so sorry, though, but I won't be able to accept these changes :/
These changes make the fetchFromApi helper do way too much, instead of concentrating the complexity in only one spot (the middleware).
fetchFromApi only exists because fetch doesn't reject promises as it should, so refactoring it was paramount to keep the semantics of the middleware.

In fact, these changes would make the use of fetchFromApi almost mandatory for everything to work together as designed.

Next time, it would be nice if you could open an issue describing what you want to be done, be it a new feature or a refactor, so we can discuss it before you (or anyone) starts coding on it.
Thanks!

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