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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status Chart: chart.js 3.6.0, improve type info, weekly updates #2324

Merged
merged 5 commits into from Nov 8, 2021

Conversation

StephanTLavavej
Copy link
Member

  • npm update.
  • README.md: Update Node.js version.
  • chart.js 3.6.0.
  • weekly_table.js updates.
  • gather_stats.ts: Improve type info.
    • Various raw fields can be null. (For author, this is when a user has been deleted; for closedAt/mergedAt, this is when a PR/issue hasn't been closed/merged.) We're already testing for null, this just acknowledges that in the type system.
    • Add types for the RateLimit and combined RawNodes that retrieve_nodes_from_network() and retrieve_nodes() return.
    • As we gradually build up the information for RateLimit, TypeScript is intelligent enough to see that spent/remaining/limit will always be number instead of null by the time we return, making the RateLimit type more useful. To achieve this, we need individual variables in retrieve_nodes_from_network(). We can say that spent/remaining/limit are number without an initializer, and TypeScript will require that we assign to them before using them. We need to change the while loop into a do/while loop so that TypeScript will see that it always runs at least one iteration (this was already guaranteed because variables.want_prs and variables.want_issues are initially true).
    • In retrieve_nodes(), we need a type assertion (as RawNodes) to tell the compiler what we're loading.
    • In warn_about_duplicates(), constructing Set<string> tells TypeScript what we're doing (otherwise we're working with Set<any>).
    • Similarly, in write_monthly_table(), we're really using a Map<string, number>.
    • After thinking about it, I've changed transform_pr_nodes() and transform_issue_nodes() to have deduced return types (with explicitly typed ret variables). This is more consistent with the rest of the code now, and while both ways are equally type-checked, the ret variables offer better diagnostics if the types are wrong (as TypeScript doesn't have to complain about the huge transformation in the function).

馃搱 Live preview: stephantlavavej.github.io/STL/

@StephanTLavavej StephanTLavavej added the documentation Related to documentation or comments label Nov 6, 2021
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner November 6, 2021 04:50
@StephanTLavavej StephanTLavavej added this to Final Review in Code Reviews Nov 6, 2021
src/gather_stats.ts Show resolved Hide resolved
src/gather_stats.ts Show resolved Hide resolved
@CaseyCarter CaseyCarter moved this from Final Review to Ready To Merge in Code Reviews Nov 8, 2021
@StephanTLavavej StephanTLavavej merged commit 7827d30 into microsoft:gh-pages Nov 8, 2021
Code Reviews automation moved this from Ready To Merge to Done Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation or comments
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants