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

Refactors 3 requests down to 1 #634

Merged
merged 6 commits into from
Mar 16, 2018
Merged

Refactors 3 requests down to 1 #634

merged 6 commits into from
Mar 16, 2018

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Mar 16, 2018

Description

Refactors the requests for title, desc, and keywords into one request to save on rate limit / overhead of each request.

Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

ui

Testing

tested frontend

@owocki owocki requested a review from mbeacom March 16, 2018 17:25
@codecov
Copy link

codecov bot commented Mar 16, 2018

Codecov Report

Merging #634 into master will decrease coverage by 0.04%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #634      +/-   ##
==========================================
- Coverage   34.12%   34.07%   -0.05%     
==========================================
  Files          90       90              
  Lines        4985     4942      -43     
  Branches      576      569       -7     
==========================================
- Hits         1701     1684      -17     
+ Misses       3217     3195      -22     
+ Partials       67       63       -4
Impacted Files Coverage Δ
app/app/urls.py 100% <ø> (ø) ⬆️
app/dashboard/helpers.py 32.5% <66.66%> (-1.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fad7cdc...cd5148c. Read the comment docs.

@owocki
Copy link
Contributor Author

owocki commented Mar 16, 2018

yay! tests passing...

@mbeacom can i get an 👀 ?

Copy link
Contributor

@mbeacom mbeacom left a comment

Choose a reason for hiding this comment

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

lgtm :shipit:

@mbeacom mbeacom added frontend This needs frontend expertise. backend This needs backend expertise. labels Mar 16, 2018
@owocki
Copy link
Contributor Author

owocki commented Mar 16, 2018

yay! note to self: it'll probably make sends to refactor the issue_detailsview at some point.. its kind of nasty

@mbeacom
Copy link
Contributor

mbeacom commented Mar 16, 2018

💯

@owocki owocki merged commit f3b4bc6 into master Mar 16, 2018
@mbeacom mbeacom deleted the kevin/refactor_3_requests branch March 16, 2018 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend This needs backend expertise. frontend This needs frontend expertise.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants