search

david-yz-liu / memory-viz

Javascript library for creating beginner-friendly memory model diagrams.
https://www.cs.toronto.edu/~david/memory-viz/demo/
2 stars 7 forks source link

Memory viz cli #43

Closed sarahsonder closed 4 months ago

sarahsonder commented 4 months ago

Proposed Changes

The purpose of this pull request is to create a command line interface for MemoryViz. This is to make it easier to run MemoryViz.

To run the CLI, run npm i to install the CLI and then run memory-viz <file-to-path>, replacing <path-to-file> with the path to a file containing MemoryViz-compatible JSON

Type of Change

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality) X
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

Before opening your pull request:

After opening your pull request:

Questions and Comments

  1. I am unsure as the why the Github tests are failing. The issue seems to be that the files/branch cannot be found. I left a more detailed message on Slack. Thank you!
  2. Since my current implementation takes in only one argument, which is the path to the file containing the Memory-Viz compatible JSON input, users do not have a choice as to where the SVG image gets saved. This also means that users cannot choose the name of the output, which means that old SVGs could get overwritten. Would you like me to change the implementation such that users may specify where they would like the image to be saved? For reference, I currently have it so that the image gets saved in the same directory in which Memory-Viz is being called. I can also look into renaming files with (1) or (2) if files of the same name already exist in a directory.
  3. Would you like me to create a new page in the docs website to document the CLI or should I do it elsewhere?
  4. Is there a particular library that you would like me to use for testing the CLI?
  5. Are we only allowing JSON files as inputs for the moment?
david-yz-liu commented 4 months ago

@sarahsonder good questions.

  1. For the output file: the current directory is fine. For the filename, take the base name of the input json and change the extension to .svg. (Future work could involve allowing the user to specify an output file.)
  2. For documentation: (1) add a brief subsection to the memory-viz/README.md with a sample invocation, and (2) yes create a new documentation page for the CLI. It can be pretty bare-bones for now, but hopefully it will grow over time as we add new options. 😄 You can use some professional pages as templates to follow, e.g. https://jestjs.io/docs/cli.
  3. You should stick with Jest testing. For now you can manually create a subprocess to execute an npx memory-viz <whatever> and check the file contents (ideally, against a snapshot!).
  4. Yes that's correct, though I'm not sure what other file types you're thinking of.
sarahsonder commented 4 months ago

Proposed Changes

The purpose of this pull request is to create a command line interface for MemoryViz. This is to make it easier to run MemoryViz.

To run the CLI, run npm i to install the CLI and then run npx memory-viz <file-to-path>, replacing <path-to-file> with the path to a file containing MemoryViz-compatible JSON

Type of Change

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality) X
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📚 Documentation update (change that only updates documentation)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

Before opening your pull request:

After opening your pull request:

Questions and Comments

I am unsure as the why the Github tests are failing. The issue seems to be that the files/branch cannot be found. I left a more detailed message on Slack. Thank you!

david-yz-liu commented 4 months ago

@sarahsonder don't put the pull request template in a comment. Instead edit the pull request description so that it contains the full information.

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9558806459

Details

Totals Coverage Status
Change from base Build 9558270699: 0.0%
Covered Lines: 388
Relevant Lines: 429
💛 - Coveralls
coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9572882702

Details

Totals Coverage Status
Change from base Build 9558270699: 0.0%
Covered Lines: 388
Relevant Lines: 429
💛 - Coveralls