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

Remove sphinx.ext.autosummary from lib/docs/conf.py #243

Merged
merged 7 commits into from
Sep 22, 2023

Conversation

ns-rse
Copy link
Collaborator

@ns-rse ns-rse commented Sep 15, 2023

Hopefully this closes #239 and the API duplication will no longer occur.

Hopefully this will prevent the duplication of API on ReadTheDocs
@ns-rse ns-rse added documentation Improvements or additions to documentation CI/CD Continuous Integration and Development labels Sep 15, 2023
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (21670ab) 69.37% compared to head (d6630ce) 69.37%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #243   +/-   ##
=======================================
  Coverage   69.37%   69.37%           
=======================================
  Files          11       11           
  Lines         493      493           
=======================================
  Hits          342      342           
  Misses        151      151           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ns-rse
Copy link
Collaborator Author

ns-rse commented Sep 15, 2023

Hmm, I think I may have been distracted and failed to put out a PR to remove the duplicate API. End-to-end tests are failing, so I merged origin/main into this branch as I recall you saying it was something you would fix @TheLostLambda but I think that may still be pending?

@TheLostLambda
Copy link
Member

@ns-rse I'll try to sort out what the CI testing is upset about. In the meantime, can we delete lib/docs/api.rst?

@ns-rse
Copy link
Collaborator Author

ns-rse commented Sep 22, 2023

@TheLostLambda thanks, wasn't paying attention, now removed.

@TheLostLambda
Copy link
Member

Let's hope this helps!

@TheLostLambda TheLostLambda merged commit ab34062 into master Sep 22, 2023
15 checks passed
@ns-rse
Copy link
Collaborator Author

ns-rse commented Sep 22, 2023

👍 What is the difference between pnpm and pnpx?

(I know little to nothing about these frameworks!).

@ns-rse ns-rse deleted the ns-rse/239-api-docs branch September 22, 2023 16:54
@TheLostLambda
Copy link
Member

The normal JS package manager is pnpm (and it allows you to run programs installed locally in the node_modules folder), and pnpx is a version that executes a remote package as a command. It installs the latest version on the fly, then runs the program.

The issue with the CI tests was that a new version of Playwright was released, so pnpx started pulling that new version, and it installed versions of the test browsers that were incompatible with the version of Playwright in our package.json. It looks like, by changing out pnpx for pnpm exec, you can run the locally installed playwright that respects the semantic versioning requirements set in the package.json.

That's how I came to understand things, anyways! Half of the CI process was using a newer version of Playwright that had been released, but the testing phase (pnpm test) used our older, locked version, so things broke.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Continuous Integration and Development documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Documentation Seems Non-Existant on ReadTheDocs
2 participants