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

Attempt to pkgify teraslice and the cli #1090

Merged
merged 4 commits into from
Apr 17, 2019
Merged

Attempt to pkgify teraslice and the cli #1090

merged 4 commits into from
Apr 17, 2019

Conversation

godber
Copy link
Member

@godber godber commented Apr 12, 2019

So the changes here add the following capabilities:

  • added a pkg build script, callable by yarn build:pkg
  • the teraslice pkg run as far as I know
    • think about ideal asset location, currently ./assets
    • assets were uploaded, data generator job to null ran
  • the teraslice-cli pkg works
    • with the exception of all assets sub commands.
      • If I omit assets init then the other commands will work
      • The init command appears to not work because it uses yeoman, which in turn uses editions and is a known issue: Error when using getmac package vercel/pkg#519. Options are:
        • implement init without yeoman.
        • Use yeoman the way as a stand alone, they way it is meant to be used. (We would implement separate yeoman generator packages and the user would use yeoman directly)
        • Address the pkg bug above.
        • "Patch" yeoman to not use editions.

Questions (other than the obvious ones from above)

  • What do we do with pkgs? Upload to github as a package as part of our release cycle?

@godber
Copy link
Member Author

godber commented Apr 16, 2019

Another option would be to re-implement the templating with hygen:

http://www.hygen.io/quick-start/

I've implemented one of their examples and built it with pkg and it functions correctly.

@godber godber changed the title WIP: Attempt to pkgify teraslice and the cli Attempt to pkgify teraslice and the cli Apr 17, 2019
@godber
Copy link
Member Author

godber commented Apr 17, 2019

I have removed the WIP in this PR. @peterdemartini, if you are good with the changes that are more global (addition of build:pkg and build script) then this can be merged. The scripts aren't meant to be the final solution, just to be a starting place for when we want to tackle this. Maybe they're not useful enough and should be removed.

The lower level changes to the asset cli code should be accepted because they are functionally equivalent and resolve pkg errors. They are the real point of merging this PR instead of just deleting it.

Copy link
Contributor

@peterdemartini peterdemartini left a comment

Choose a reason for hiding this comment

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

LGTM

@peterdemartini peterdemartini merged commit 257b899 into master Apr 17, 2019
@peterdemartini peterdemartini deleted the pkg-ify-1 branch April 17, 2019 18:17
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