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.

Sign up for GitHub

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

Jump to bottom

SKEDGE-14 #comment convert to async/await instead of .then() logics, organizing awaits #30

Merged
merged 8 commits into from
Jun 9, 2023
Merged

SKEDGE-14 #comment convert to async/await instead of .then() logics, organizing awaits #30

merged 8 commits into from
Jun 9, 2023

Conversation

demonlexe
Copy link
Contributor

SKEDGE-14 started; still need to do more testing

@demonlexe demonlexe marked this pull request as ready for review March 28, 2023 00:59
@heartgg heartgg changed the base branch from main to develop March 28, 2023 20:31
@heartgg heartgg changed the base branch from develop to main March 28, 2023 20:33
@heartgg heartgg changed the base branch from main to develop March 28, 2023 20:33
heartgg
heartgg suggested changes Mar 28, 2023
View reviewed changes
Copy link
Member

@heartgg heartgg left a comment

Choose a reason for hiding this comment

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

Good work, I assume it was tested and works.
Only needs some minor changes that are not really related to the changes, just mostly for maintainable code

src/data/fetchFromRmp.ts Outdated Show resolved Hide resolved
src/data/fetchFromRmp.ts Outdated Show resolved Hide resolved
src/data/fetchFromRmp.ts Outdated Show resolved Hide resolved
@demonlexe
SKEDGE-14 #comment add function to group properties, use this functio…
b5fcd34 
…n for the RMP fetches to avoid 504 error.

#comment address requests from Adam Szumski
#comment add new interfaces for the Professor Rating object
@demonlexe
move functions, add comments
2dd8451
@demonlexe demonlexe marked this pull request as draft March 29, 2023 01:17
@demonlexe
add reportError function, to be modified later; use fetchRetry for in…
d8b9bc4 
…itial fetches of professors from RMP
@demonlexe
SKEDGE-14 #comment modularize retry logic inside validateResponses fu…
dfe3809 
…nction, instead of grouping fetches.

(This way seems shorter and safer)
@demonlexe demonlexe marked this pull request as ready for review March 29, 2023 01:59
@demonlexe demonlexe requested a review from heartgg March 29, 2023 01:59
@heartgg heartgg merged commit 5ac7669 into UTDNebula:develop Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heartgg heartgg Awaiting requested review from heartgg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@demonlexe @heartgg