Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Add yarn as alternative --with-yarn #256

Merged
merged 1 commit into from
Feb 21, 2017
Merged

Add yarn as alternative --with-yarn #256

merged 1 commit into from
Feb 21, 2017

Conversation

fooflare
Copy link
Contributor

As I mentioned in #187 I made some changes to have yarn as option with --with-yarn flag. It works following the specifications below:

  1. With new:
    1.1. If flag is specified:

    • Checks if yarn is installed and if not, installs it and changes the default command from npm to yarn (adding equivalent arguments)
    • Inits the yarn project, downloads and installs webpack dependencies

    1.2. If not, npm is executed

  2. With generate webpack, I added the same flag as alternative:
    2.1. Checks if yarn.lock already exists and if so, yarn is established as default
    2.2. If yarn.lock doesn't exist:

    • If flag was specified, yarn is selected (same process than with new command)
    • If not, npm is executed

@markbates
Copy link
Member

markbates commented Feb 21, 2017

This looks good, but to be feature complete it needs to support the following commands:

  • buffalo new
  • buffalo dev
  • buffalo build

Those commands are all touch the "asset pipeline".

I also think we should rename --skip-webpack to --skip-assets.

@fooflare
Copy link
Contributor Author

One thing I haven't clear is about dev and build. I checked both but is webpack directly executed, not npm. Or am I wrong?

In reference to the renaming, I am not sure if --skip-assets or maybe --skip-npm. In this PR if webpack is skipped, yarn is skipped too, even if the flag is specified (no dependencies to be generated). And assets are generated in public in any case (application.js/.css and logo), so I am not sure if it would be totally clear.

@markbates markbates added the enhancement New feature or request label Feb 21, 2017
@markbates markbates added this to the 0.8.0 milestone Feb 21, 2017
@markbates markbates self-assigned this Feb 21, 2017
@markbates
Copy link
Member

Ok, I tested this and validated that all three commands work with a yarn application. Fantastic. Thanks for this.

@markbates markbates merged commit a37b69f into gobuffalo:master Feb 21, 2017
@fooflare
Copy link
Contributor Author

Don't thank me, I did very few. Thanks to you for all your efforts in the project.

@markbates
Copy link
Member

It's a group effort! :)

@philipithomas
Copy link
Contributor

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants