bitcoinjs-lib icon indicating copy to clipboard operation
bitcoinjs-lib copied to clipboard

✨feature: add doc

Open jasonandjay opened this issue 1 year ago • 14 comments

Add bitcoinjs-lib documentation

  • [x] Automatically read ts file and generate
  • [x] Automatically triggered by github actions (github pages need to be enabled)

jasonandjay avatar Sep 29 '23 09:09 jasonandjay

TODO:

  • Figure out why deploy-doc is broken.
  • Add more comments to various items in the library so that the docs are more fleshed out.

junderw avatar Sep 29 '23 17:09 junderw

TODO:

  • Figure out why deploy-doc is broken.
  • Add more comments to various items in the library so that the docs are more fleshed out.

I guess it may caused by get pages token failed. You can enable github pages first and select source from github actions, then you can re-run failed job.

jasonandjay avatar Sep 30 '23 00:09 jasonandjay

Still not working.

This is the yaml file it recommended me when I tried to configure a static HTML setup through Github Actions.

Perhaps something is missing from your diff? Could you look into it for me?

# Simple workflow for deploying static content to GitHub Pages
name: Deploy static content to Pages

on:
  # Runs on pushes targeting the default branch
  push:
    branches: ["master"]

  # Allows you to run this workflow manually from the Actions tab
  workflow_dispatch:

# Sets permissions of the GITHUB_TOKEN to allow deployment to GitHub Pages
permissions:
  contents: read
  pages: write
  id-token: write

# Allow only one concurrent deployment, skipping runs queued between the run in-progress and latest queued.
# However, do NOT cancel in-progress runs as we want to allow these production deployments to complete.
concurrency:
  group: "pages"
  cancel-in-progress: false

jobs:
  # Single deploy job since we're just deploying
  deploy:
    environment:
      name: github-pages
      url: ${{ steps.deployment.outputs.page_url }}
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Setup Pages
        uses: actions/configure-pages@v3
      - name: Upload artifact
        uses: actions/upload-pages-artifact@v2
        with:
          # Upload entire repository
          path: '.'
      - name: Deploy to GitHub Pages
        id: deployment
        uses: actions/deploy-pages@v2

junderw avatar Sep 30 '23 01:09 junderw

Looks like actions/configure-pages is missing?

junderw avatar Sep 30 '23 01:09 junderw

Looks like actions/configure-pages is missing?

logger report [Branch "doc" is not allowed to deploy to github-pages due to environment protection rules.] To allow the "doc" branch to deploy to GitHub Pages, you'll need to adjust the branch protection rules. Follow these steps:

Go to your GitHub Repository: Open the repository in your browser.

Click on "Settings": It's usually located near the right end of the menu bar, next to "Actions" and "Insights".

Navigate to "Branches": In the left sidebar, click on the "Branches" tab.

Select "Add rule": You should see an option to add a new branch protection rule. Click on it.

Enter "doc" as the branch name: In the "Branch name pattern" field, type "doc" to specify the branch you want to adjust the rules for.

Configure Protection Rules: Here, you'll have several options. You'll want to pay attention to the following:

Require pull request reviews before merging: If you have this enabled, make sure it allows merging without approval. You might need to disable it temporarily for the "doc" branch.

Require status checks to pass before merging: Ensure that the necessary checks (e.g., GitHub Pages deployment) are not required for the "doc" branch.

Include administrators: Make sure this option is checked if you want administrators to be exempt from these rules.

Save Changes: After making the necessary adjustments, click on the "Save changes" button.

jasonandjay avatar Sep 30 '23 01:09 jasonandjay

Ok, well, I can verify the results of the doc command locally. Perhaps we should make the doc jobs only run on master.

Having it run on pull requests will just pollute the CI with failed tests.

junderw avatar Sep 30 '23 15:09 junderw

Deploy doc task has updated, it will only take effect on master branch.

jasonandjay avatar Oct 02 '23 18:10 jasonandjay

I wonder about versioning.

If it would be possible to have it recognize whether it's being deployed on master or a tag and change the URL path?

Then maybe people could navigate to older versions by clicking a link to the docs generated for a specific tag.

I would hope that the default page is the last tagged version or something.

If that's not possible, I guess it's fine.

I'd also like to flesh out some of the documentation in this PR so that the first release of the docs isn't so bare bones.

If you can help by documenting any of the smaller stuff I'd appreciate it. The more the better.

junderw avatar Oct 02 '23 20:10 junderw

I think add version control is possible and necessary but with low priority. And flesh out documentation is how priority. I want to add description and examples to function、class、interface and namespace. examples can be copied from tests, so users can easy to use and it`s better for us to maintain.

such as fromBase58Check, do you think any other content or content style do we need to add? demo

jasonandjay avatar Oct 05 '23 06:10 jasonandjay

For properties, we can add description like this, do you think its ok? proper

jasonandjay avatar Oct 05 '23 06:10 jasonandjay

I want to add description and examples to function、class、interface and namespace. examples can be copied from tests, so users can easy to use and it`s better for us to maintain.

such as fromBase58Check, do you think any other content or content style do we need to add?

This looks fine. I am a little worried about tests and their copy-pasted doc-equivalents going stale... but I think as long as I remember to mention fixing the doc strings in review it should be fine.

I think add version control is possible and necessary but with low priority.

Yeah, this is a "nice to have"

You are awesome, thanks for your help!

junderw avatar Oct 05 '23 07:10 junderw

Got it typedoc-plugin-bitcoinjs-runcase is a devDependencies, so it will not intrude into our program and help provide a run code environment when building documents.

It will now be more efficient for us to improve the documentation

Or, let’s add the key comments in the document first, and then add the run case. What do you think?

jasonandjay avatar Nov 05 '23 02:11 jasonandjay

I did two things

  1. ⏪ rollback: separate typedoc-plugin-bitcoinjs-runcase to another PR
  2. Add key comments to the code to make the document look more complete.

jasonandjay avatar Feb 29 '24 18:02 jasonandjay

Thanks. That lowers the scope of the review significantly. I'll take another look later today.

junderw avatar Mar 01 '24 01:03 junderw

Is there anything that needs to be optimized? Maybe we can push document construction to next step.

jasonandjay avatar Mar 06 '24 10:03 jasonandjay

Deployed successfully!

https://bitcoinjs.github.io/bitcoinjs-lib/

Maybe add this to the README?

junderw avatar Mar 06 '24 10:03 junderw