Skip to content

Documentation update pass #23

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

Merged
merged 6 commits into from
Oct 25, 2022
Merged

Documentation update pass #23

merged 6 commits into from
Oct 25, 2022

Conversation

dshemetov
Copy link
Contributor

@dshemetov dshemetov commented Mar 31, 2022

Part of #13.

Testing the API endpoints:

  • each endpoint has a link to the API docs
  • most endpoints have an example call that works

Endpoints tested:

@dshemetov dshemetov force-pushed the ds/doc-updates branch 2 times, most recently from 28c17bf to 9fb8339 Compare April 4, 2022 20:28
@brookslogan
Copy link
Contributor

Note this will need a re-document() when ready. (From the checklist, it looks pretty ready; can the remaining boxes be left for another issue?)

@brookslogan
Copy link
Contributor

This still needs a re-document(). And an issue to address the remaining checklist items.

@brookslogan
Copy link
Contributor

Closes #50.

@brookslogan brookslogan self-requested a review as a code owner October 19, 2022 20:33
@brookslogan
Copy link
Contributor

Looks like

  • the first CI run with the updated documentation timed out trying to test the examples
  • locally a check() runs fine on my system
  • locally a devtools::run_examples() is currently stuck at the covid_hosp_facility_lookup example; not sure if it's going to terminate or not.

Some potential causes:

  • an API hiccup
  • a GitHub Action configuration issue
  • a bug in the examples

Copy link
Contributor

@brookslogan brookslogan left a comment

Choose a reason for hiding this comment

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

Not sure how I self-requested review. The doc changes looked good when I looked them over before. The re-document() revealed an (inconsistent) issue with examples taking a long time / never terminating, maybe due to sometimes-long covid_hosp_facility_lookup run times; I'm following up on this on Slack. I think it's good to either merge or to wait for some findings regarding what's causing the inconsistent long run times. @keanmingtan would it slow you down at all to continue development off a branch split off from this PR's branch?

@brookslogan
Copy link
Contributor

Trying out just marking this + another long-running example \donttest. CI checks are still going to run them, so if it's this causing them to time out, we may need to change this to something even more restrictive until the covid_hosp_facility_lookup backend performance improvements and a cmu-delphi/delphi-epidata#48 fix are out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants