added check function for maximum page count of issue list #855
Conversation
|
if this change is acceptable for gap-analysis also, I'd propose to have shared routine on fetching list of issues as another js file. |
|
Is this still active? Can we merge it or close it? |
|
@aphillips sorry for taking time to reply. I've took time to resolve errors in my local branch while git-rebase to gh-pages:HEAD, to check whether this modification would work with the latest script. I believe this fix to extend max retrieval of issue lists from github is valid even now, and also we may need this relatively soon, since we already have 488 issues listed in the page, while current hard coded max as 500 issues (5 pages with 100 issues each). |
reviews/index.html
Outdated
| @@ -415,7 +433,10 @@ <h2 class="notoc">Filter results</h2> | |||
| document.getElementById('total').textContent = "There are "+filteredIssues+" issues." | |||
| } | |||
| </script> | |||
| <script>window.onload = getAllData()</script> | |||
| <script> | |||
| // window.onload = getAllData() | |||
There was a problem hiding this comment.
remove dead code (it's why we have version control)
reviews/index.html
Outdated
| function getAllData (maxpages_new) { | ||
| for (var p=1;p<maxpages_new+1;p++) fetchIssues(p) |
There was a problem hiding this comment.
Use a better name? Perhaps pages? Or totalPages? Why don't we just replace the value of maxpages with a call to fetchIssuesCount()?
There was a problem hiding this comment.
hrm.. since this PR was something of quick hack, so to remove old variable, I think I should rewrite this part with Promise.all.
Please wait a bit more for rewriting.
✅ Deploy Preview for i18n-activity ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@aphillips sorry for taking time. |
for #619
change is to check maximum page count using HEAD before accessing API with GET
this also make enable to reduce number of access to API with GET when total number of issues is lower than 401 (might be useful for gap-analysis??).