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

Refactor #207

Closed
wants to merge 13 commits into from
Closed

Refactor #207

wants to merge 13 commits into from

Conversation

pablomendezroyo
Copy link
Contributor

@pablomendezroyo pablomendezroyo commented Apr 8, 2022

Context

DAppNodeSDK is a critical dappnode tool used daily in Github actions. It has not received maintenance from a long time ago and some things are out of date.

Furthermore, there is code duplicated between dappmanager and the SDK:

  • Docker-compose safe keys
  • Manifest schema
  • Setup wizard schema

This code duplication makes the developing experience annoying and results in production errors due to missed updates to both repositories

Approach

The SDK should contain all the types and validating functions, then the dappmanager should use the DAppNodeSDK as a dependency and import these types and validating functions to be used in during the packages installation. This will benefit DAppNode in a better development experience, fewer bugs in production, easier implementation for new wizard/manifest/compose keys features, etc.

Roadmap

  • Set types in the DAppNodeSDK (Take into account that the dappmanager joins all the release files, this will result in a manifest type with the added keys setupWizard, gettingStarted...)
  • Update any dependencies if required
    • ajv & ajv-errors
    • typescript
  • Update Manifest schema and remove deprecated keywords
  • Implement schemas
  • Implement validating functions:
  • Implement tests for all the schemas:
  • Edit official docker-compose schema according to dappnode requirements. Take into account safekeys from dappmanager
  • Move unsafeCompose from dappmanage to SDK
  • Abstract code from wizard manifest and compose for:
  • Use the DAppNodeSDK as a dependency in the dappmanager. Consider using json-schema-to-ts tool to infer types. Use of yarn link to use this PR as a dependency
  • Link docs with schemas

@pablomendezroyo pablomendezroyo marked this pull request as draft April 8, 2022 15:53
@pablomendezroyo pablomendezroyo force-pushed the pablo/dappmanager-dependency branch from 62edf47 to 328eac1 Compare April 13, 2022 09:11
@github-actions
Copy link

github-actions bot commented Apr 13, 2022

DAppNode bot has built and pinned the release to an IPFS node, for commit: 232d061

This is a development version and should only be installed for testing purposes, install link

/ipfs/QmTYimGsGeGwn71H2QH7GTcWPAKrxzSukWhN956ha57C8o

(by dappnodebot/build-action)

@pablomendezroyo pablomendezroyo force-pushed the pablo/dappmanager-dependency branch from c538381 to b2bf8e2 Compare April 13, 2022 10:45
setup manifest test

Setup processError as generic function

Implementing compose schema tests

fixed compose validation

implement setupwizard tests

fix typo

fix typo

fix test init and build
abstracted writeReleaseFile for compose manifest and wizard
.github/ISSUE_TEMPLATE/compose_feature.md Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
src/commands/githubActions/bumpUpstream/index.ts Outdated Show resolved Hide resolved
@@ -115,9 +102,6 @@ export const releaseFilesDefaultNames: {
compose: "docker-compose.yml",
avatar: "avatar.png",
setupWizard: "setup-wizard.json",
setupSchema: "setup.schema.json",
setupTarget: "setup-target.json",
setupUiJson: "setup-ui.json",
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot merge this change until there's hard proof that no package is using the old setup wizard v1 feature

src/releaseFiles/validateSchema.ts Outdated Show resolved Hide resolved
src/releaseFiles/validateSchema.ts Outdated Show resolved Hide resolved
src/releaseFiles/validateSchema.ts Outdated Show resolved Hide resolved
@pablomendezroyo pablomendezroyo force-pushed the pablo/dappmanager-dependency branch 2 times, most recently from e05e0ae to 96498af Compare May 19, 2022 12:02
abstracted getReleaseFilePath

abstracted readReleaseFile

file structure reorg

add issues template for compose setupwizard and manifest

reorg code

set release automatically on schema changes

set sdk releases manually

Setup compose schema

relocate types

defined specs and added types

Move compose validation to sdk

stop progress

add safeCompose

added parseComposeToProduction

remove issues_templates

set workflow old

bump node ci version

change readReleasFile types return

fix tests

fix

use a new global ajv instance for each releaseFIle

add ajv errros
@pablomendezroyo pablomendezroyo force-pushed the pablo/dappmanager-dependency branch from 0c58520 to 232d061 Compare May 19, 2022 13:09
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