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

Initial implementation #3

Merged
merged 34 commits into from
Apr 9, 2021
Merged

Initial implementation #3

merged 34 commits into from
Apr 9, 2021

Conversation

rytswd
Copy link
Contributor

@rytswd rytswd commented Apr 7, 2021

Started the initial implementation by providing 3 commands

Available commands

  • importer preview FILENAME: see what's going to happen
  • importer generate FILENAME: check all Importer annotations and retrieve the referenced files
  • importer purge FILENAME: remove any data between annotation pairs

Other information

  • GoReleaser is used to create the CLI available
  • Homebrew Tap is created separately at github.com/upsidr/homebrew-tap, which allows the CD setup here to generate the necessary Formula upgrade; I currently get 404 probably because this repo is private, but once made public, we should be able to run brew install upsidr/tap/importer
  • testdata directory contains some files that are used in Go test cases

Known limitations

  • File reference is done by current directory of where the CLI is run, not relative to the file. This makes it difficult to use, and should be fixed soon. Fixed, the import is done based on relative path from the file with Importer annotation
  • Original implementation works with Markdown only.
  • No styling such as triple-backtick or quotes, which should be provided as an option as a part of Importer annotation.
  • README.md is updated using Importer, but there is no automation setup for it; we should be running Importer against all the key files such as README.md - or perhaps all the files in this repo (the performance is not great at the moment, though).

Other considerations to make

  • importer preview FILENAME should give the preview of dependency graph and action plans; the current debug print should be done via --dry-run flag instead
  • Some parts are using byte slice, and some using string; we should be able to stick to one - I think string should suffice most use cases
  • Package structure isn't the best I found - it works, but there are too many public elements, which we should be able to tighten much more
  • This is using Cobra for the CLI support, but it seems to be an overkill for something as simple as Importer. We should support flag, subcommand, and autocomplete - those are the basic features we should be able to find, and doesn't probably need full fledge Cobra support.

@rytswd
Copy link
Contributor Author

rytswd commented Apr 7, 2021

Test code is to be added soon

@rytswd rytswd marked this pull request as ready for review April 8, 2021 23:04
@rytswd rytswd requested a review from sryoya April 8, 2021 23:05
@rytswd
Copy link
Contributor Author

rytswd commented Apr 8, 2021

Although some code doesn't have the coverage, test code has been added to cover most. This is ready for review, and also I have preliminarily created a release v0.0.1-alpha for Homebrew testing based off of this branch. I will replace that once this PR is merged.

Copy link
Member

@sryoya sryoya left a comment

Choose a reason for hiding this comment

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

LGTM except typo

internal/parse/annotatinos.go Outdated Show resolved Hide resolved
internal/parse/parse.go Outdated Show resolved Hide resolved
@rytswd rytswd merged commit b59832f into main Apr 9, 2021
@rytswd rytswd deleted the initial-implementation branch April 9, 2021 17:20
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