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

Status Chart: Add Combined PR Age and GitHub API script #1056

Merged
merged 5 commits into from
Jul 20, 2020
Merged

Status Chart: Add Combined PR Age and GitHub API script #1056

merged 5 commits into from
Jul 20, 2020

Conversation

StephanTLavavej
Copy link
Member

Add GitHub API script, daily table, Combined PR Age chart.

Fixes #1043.

  • gather_stats.js
    • This uses the GitHub API to gather PR/issue statistics. It runs on the console (not in the browser), using the latest version of Node.js (currently 14.5.0). I've significantly improved it since my initial version in Status Chart: Add GitHub PR/issue/bug data #1038 that wasn't checked into the repo:
    • This loads a personal access token from a .env file, avoiding the danger of hardcoding it in the script.
    • Because it currently takes about 20 seconds to list all PRs/issues through the GitHub API, the script displays a command-line progress bar.
    • For efficiency, the script transforms PR/issue data as it's received (instead of repeatedly during analysis).
    • The script now analyzes the data with daily resolution. Accordingly, I've adjusted the beginning date.
    • The script now writes a file, daily_table.js, complete with a "generated file" warning comment.
    • The script now calculates combined PR age.
  • daily_table.js
    • The generation process is stable, so regenerating this file will typically add rows to the bottom with no other changes. However, it's possible for previous rows to change (e.g. due to relabeling or reopening).
  • weekly_table.js
    • I extracted the classic table, still updated manually, for symmetry. This also makes the source and history of index.html easier to read. Eventually, I plan to generate the cxx20 and lwg stats, and we'll stop tracking vso after porting all old bugs, leaving only libcxx that can't be easily generated yet.
  • .gitignore
    • We need to ignore the node_modules directory that npm creates. We also need to ignore the .env file, but I can't tell you why - it's a secret. 😹
  • package.json
    • This is generated by npm init and is updated when dependencies are installed. I filled out some reasonable information in response to its questions, but I don't intend to publish this in the registry (as it is hardcoded for our repo and not intended for general use, although it could serve as the basis for such work).
  • package-lock.json
    • This is also a generated file, and I don't really understand its purpose, except that it's supposed to be checked in (something about pinning the exact versions of all transitive dependencies).
  • index.html
    • Add hr styling, to separate the two charts.
    • Include daily_table.js and weekly_table.js.
    • Move the main <script> into the <head>, making it easier to read the logic versus the content. (This required only one change, moving the "Toggle Timeframe" button's addEventListener to window.onload, which is invoked after the page has finished loading.)
    • Generalize get_values because there are two tables now.
    • Rename chart_data to status_data (and similarly for chart_options) because there are two charts now.
    • We don't need labels when data is Point[].
    • Rename 'meow-axis' to 'meowAxis'. This will make Chart.js v3 migration easier, where axes are specified with options.scales.meowAxis.
    • Add age_data.
    • Give timeframe_all and timeframe_github names, making it easier to refer to them later (timeframes[0] was unnecessarily mysterious).
    • Extract common_options used by both charts.
    • Set borderJoinStyle to 'round'. The default 'miter' makes rapidly oscillating lines too spiky. Also set borderCapStyle to 'round'. It's a very slight visual improvement when the end of a line is increasing or decreasing.
    • Extract a make_xAxes function, as both charts use the same x-axis, but with different timeframes.
    • ...common_options is "spread syntax".
    • Add age_options for the new chart. Note that attempting to centralize their title configuration (where display and fontSize are the same, but text is different) seems like more trouble than it's worth.
    • Construct the age_chart, and move the button logic as previously mentioned.
    • Add the <canvas id="ageChart"> followed by an explanation.

Live preview: stephantlavavej.github.io/STL/

@StephanTLavavej
Rename 'meow-axis' to 'meowAxis'.
e684909 
This will make Chart.js v3 migration easier,
where axes are specified with options.scales.meowAxis.
@StephanTLavavej
Set borderJoinStyle to 'round'.
6153f1e 
The default 'miter' makes rapidly oscillating lines too spiky.

Also set borderCapStyle to 'round'. It's a very slight visual
improvement when the end of a line is increasing or decreasing.
@StephanTLavavej
2020-07-17 update.
094a5ce
@StephanTLavavej
We don't need labels when data is Point[].
a74aad8
@StephanTLavavej
Add GitHub API script, daily table, Combined PR Age chart.
06e958d 
Fixes microsoft#1043.

* `gather_stats.js`
  + This uses the GitHub API to gather PR/issue statistics. It runs on the console (not in the browser), using the latest version of Node.js (currently 14.5.0). I've significantly improved it since my initial version in microsoft#1038 that wasn't checked into the repo:
    - This loads a personal access token from a `.env` file, avoiding the danger of hardcoding it in the script.
    - Because it currently takes about 20 seconds to list all PRs/issues through the GitHub API, the script displays a command-line progress bar.
    - For efficiency, the script transforms PR/issue data as it's received (instead of repeatedly during analysis).
    - The script now analyzes the data with daily resolution. Accordingly, I've adjusted the beginning date.
    - The script now writes a file, `daily_table.js`, complete with a "generated file" warning comment.
    - The script now calculates combined PR age.
* `daily_table.js`
  + The generation process is stable, so regenerating this file will typically add rows to the bottom with no other changes. However, it's possible for previous rows to change (e.g. due to relabeling or reopening).
* `weekly_table.js`
  + I extracted the classic table, still updated manually, for symmetry. This also makes the source and history of `index.html` easier to read. Eventually, I plan to generate the `cxx20` and `lwg` stats, and we'll stop tracking `vso` after porting all old bugs, leaving only `libcxx` that can't be easily generated yet.
* `.gitignore`
  + We need to ignore the `node_modules` directory that `npm` creates. We also need to ignore the `.env` file, but I can't tell you why - it's a secret.
* `package.json`
  + This is generated by `npm init` and is updated when dependencies are installed. I filled out some reasonable information in response to its questions, but I don't intend to publish this in the registry (as it is hardcoded for our repo and not intended for general use, although it could serve as the basis for such work).
* `package-lock.json`
  + This is also a generated file, and I don't really understand its purpose, except that it's supposed to be checked in (something about pinning the exact versions of all transitive dependencies).
* `index.html`
  + Add `hr` styling, to separate the two charts.
  + Include `daily_table.js` and `weekly_table.js`.
  + Move the main `<script>` into the `<head>`, making it easier to read the logic versus the content. (This required only one change, moving the "Toggle Timeframe" button's `addEventListener` to `window.onload`, which is invoked after the page has finished loading.)
  + Generalize `get_values` because there are two tables now.
  + Rename `chart_data` to `status_data` (and similarly for `chart_options`) because there are two charts now.
  + Add `age_data`.
  + Give `timeframe_all` and `timeframe_github` names, making it easier to refer to them later (`timeframes[0]` was unnecessarily mysterious).
  + Extract `common_options` used by both charts.
  + Extract a `make_xAxes` function, as both charts use the same x-axis, but with different timeframes.
  + `...common_options` is "spread syntax".
  + Add `age_options` for the new chart. Note that attempting to centralize their `title` configuration (where `display` and `fontSize` are the same, but `text` is different) seems like more trouble than it's worth.
  + Construct the `age_chart`, and move the button logic as previously mentioned.
  + Add the `<canvas id="ageChart">` followed by an explanation.
@StephanTLavavej StephanTLavavej added the documentation Related to documentation or comments label Jul 19, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner July 19, 2020 09:57
@StephanTLavavej StephanTLavavej linked an issue Jul 19, 2020 that may be closed by this pull request
Status Chart: Track the combined age of the PR backlog #1043
Closed
@StephanTLavavej StephanTLavavej merged commit 4245a54 into microsoft:gh-pages Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Assignees
No one assigned
Labels
documentation Related to documentation or comments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Status Chart: Track the combined age of the PR backlog
2 participants
@StephanTLavavej @CaseyCarter