Skip to content

Refactor app, build and tests#125

Merged
jochenklar merged 24 commits intomainfrom
dev
Apr 17, 2026
Merged

Refactor app, build and tests#125
jochenklar merged 24 commits intomainfrom
dev

Conversation

@jochenklar
Copy link
Copy Markdown
Member

@jochenklar jochenklar commented Apr 9, 2026

This PR aims to refactor the protocol to make it better maintainable. In particular:

  • Use vite to build the front end.
  • Remove the separate app/Makefile and move JavaScript config into the root of the repo.
  • Remove assets.py build script and only use vite to build the (web) app.
  • Update the README.md.
  • Update all other build scripts to use Path.rglob instead of os.walk and add logging.
  • Update .pre-commit-config.yaml and apply ruff-format and eslint rules.
  • Convert the tree files to yaml.
  • Fix missing properties in schema files.
  • Fix tests to actually check pattern, schema, and tree files.

Draft version: https://draft.isimip.org/protocol/dev/.

@jochenklar jochenklar marked this pull request as ready for review April 12, 2026 11:02
@jochenklar jochenklar requested review from jurikane and thiasB April 12, 2026 11:03
@jochenklar jochenklar self-assigned this Apr 12, 2026
Copy link
Copy Markdown
Member

@jurikane jurikane left a comment

Choose a reason for hiding this comment

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

Reviewed overall and left two non-blocking inline comments/questions; otherwise looks good to me.

Comment thread build/tree.py
# step 3: write json file
write_json(output_path, tree_json)
# write tree as json
write_json(output_path, tree)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it on purpose to still write it as JSON now that the files are .yaml ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, the output is always in JSON to be easily consumable by tools. We switched the input to yaml one or two years ago to make editing simpler.

Comment thread README.md
make dev # like make, but lining the front-end assets for development

make serve # starts a http server on port :8000 so that you can access the protocol in your browser
make app
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe we can add a short troubleshooting note for branch switches or dependency changes,
I hit a build failure until I removed node_modules and reran npm ci.
Something like:
make cleannode && make app

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I changed the Makefile to run npm ci on each npm call and added some information about the clean targets to README.md.

Comment thread README.md Outdated
make watch # automatically rebuild when the source changes
```

When working with different branches or after dependency changed, it the following make targets can be used to clean the local copy:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"dependency changed, it the"
to
"dependency changes, the"

@jochenklar jochenklar merged commit a88205a into main Apr 17, 2026
2 checks passed
@jochenklar jochenklar deleted the dev branch April 17, 2026 10:21
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.

3 participants