Skip to content

Conversation

@kdelap00
Copy link

Added a .yml file that defines a github action for update the documentation of the sdk.
The action uses a js script to take the options of the sdk commands and create a pull request to the doc repository

@kdelap00 kdelap00 requested a review from a team as a code owner November 14, 2023 11:34
Copy link
Contributor

@pablomendezroyo pablomendezroyo left a comment

Choose a reason for hiding this comment

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

I

run: npm install -g @dappnode/dappnodesdk@${{ github.event.release.tag_name }}

- name: Download script from sdk
run: wget https://raw.githubusercontent.com/dappnode/DAppNodeSDK/master/.github/workflows/scripts/docs.cjs -O docs.cjs
Copy link
Contributor

Choose a reason for hiding this comment

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

This action is weird, you are getting the script from upstream instead of from local

repository: dappnode/DAppNodeDocs

- name: Set up dappnodesdk
run: npm install -g @dappnode/dappnodesdk@${{ github.event.release.tag_name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird, you are getting the sdk from npm instead of from local, changes in branches will not make an effect in gha until merged and release if following this approach.


- name: Commit to docs
run: |
git add docs/dev/sdk/commands.md
Copy link
Contributor

Choose a reason for hiding this comment

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

there are more docs to be tracked of the sdk not only commands:

We should first ensure that the dappnode sdk contains all this information(commands, github actions and files references), and include this information somehow in the release assets. This is an issue on its own. The release assets could be:

  • commands: .js files
  • files references: .json files
  • github actions: .yml files

git commit -m "Updated docs for new SDK release"

- name: Pull request to docs
uses: peter-evans/create-pull-request@v5
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont like the idea of having a repository A making PRs on a repository B unless is strictly necessary to do so. Instead I would follow a different approach:

The dappnodeSDK should contain in its releases the necessary assets for the documentation repository to document the dappnodeSDK. The dappnode documentation could have as a dependency the dappnodeSDK, using the exports commands.js and files references (manifest-schema.json, setupWizard-schema.json) to create the sdk documentation. For the Github actions there should be used a different approach as long as they are .yml files. Consider parsing and exporting the github actions yaml files in .json format so they can be exported and used as a javascript dependency too in the dappnode documentation repo

@@ -0,0 +1,71 @@
const { exec } = require('child_process');
Copy link
Contributor

Choose a reason for hiding this comment

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

Use esm modules instead of commonjs when possible

@@ -0,0 +1,71 @@
const { exec } = require('child_process');
const fs = require('fs/promises');
Copy link
Contributor

Choose a reason for hiding this comment

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

Its weird to have a scripts folder just for this purpose in the dappnode sdk

@pablomendezroyo
Copy link
Contributor

Wont implement

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.

2 participants