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

Add npm installer #288

Merged
merged 22 commits into from
Mar 26, 2017
Merged

Add npm installer #288

merged 22 commits into from
Mar 26, 2017

Conversation

rtfeldman
Copy link

@rtfeldman rtfeldman commented Jan 7, 2017

To try it out: (this will install elm-format 0.5.2-alpha for Elm 0.18)

$ npm install -g /path/to/elm-format/package/npm
$ elm-format --version

Should work on the 64-bit versions of:

  • macOS
  • Linux
  • Windows

Notes

It installs elm-format versions based on the version specified in package.json, which is set up to include both the elm-format version and the Elm version.

For example, to explicitly get the latest version, you'd run npm install -g elm-format@0.5.2-elm0.18.0 (although in the case of the absolute latest version you could always just do npm install -g elm-format)

This means when publishing a new version, you'd need to publish all the combinations as separate npm package releases.

TODO

Add CI tests like what elm-platform uses, to verify all the combinations download and install successfully.

@rtfeldman
Copy link
Author

@avh4 let me know what your npm username is and I'll add you as an owner to the elm-format package.

@avh4
Copy link
Owner

avh4 commented Jan 7, 2017

@avh4
Copy link
Owner

avh4 commented Jan 7, 2017

for version numbers, I think maybe we should only publish the elm-0.18 version via npm (it can be used for any other supported elm-version by passing --elm-version=...

and if we do want to publish other elm version versions, we could publish them as elm-format-0.17@0.5.2, etc.

What do you think of that?

@avh4 avh4 added this to the 0.6.0 public beta milestone Jan 7, 2017
@avh4 avh4 added the packaging label Jan 7, 2017
@rtfeldman
Copy link
Author

if we do want to publish other elm version versions, we could publish them as elm-format-0.17@0.5.2, etc.

Well, there's a slight risk of someone being a jerk and squatting on the package name before we can grab it. 😉

I presume the upside would be that you could do npm install elm-format@0.5.2? It's possible to support that as an additional release on top of elm-format@0.5.2-elm0.18.0 😄

@rtfeldman
Copy link
Author

I guess there's also the issue that this would result in a lot of versions being listed when you looked at the module. Arguably that's a good thing though; would let you see all the supported combinations all in one place. 🤔

@avh4
Copy link
Owner

avh4 commented Jan 7, 2017

Ah, okay. Well I think we should start with only publishing elm-format that defaults --elm-version to the latest elm version, so elm-format@5.2.0. And if there's later a good reason to publish multiple versions, we can decide between the version suffix and publishing multiple packages.

@rtfeldman
Copy link
Author

Wouldn't elm-format@0.5.2 be more appropriate than 5.2.0?

@avh4
Copy link
Owner

avh4 commented Jan 8, 2017 via email

@rtfeldman
Copy link
Author

@avh4 I made the package change, so I think the only thing remaining is to add some quick Travis tests to show that it installs and runs.

What I've done for these in the past is to write .js scripts to test them, but I see your Bash-fu is better than mine. How would you like to proceed?

@bobbypriam bobbypriam mentioned this pull request Mar 9, 2017
@rtfeldman rtfeldman changed the title WIP: Add npm installer Add npm installer Mar 14, 2017
@rtfeldman rtfeldman changed the title Add npm installer WIP: Add npm installer Mar 14, 2017
@avh4 avh4 self-assigned this Mar 26, 2017
var packageInfo = require(path.join(__dirname, "package.json"));

// Use major.minor.patch from version string - e.g. "1.2.3" from "1.2.3-alpha"
var binVersion = packageInfo.version.match(/^(\d+\.\d+\.\d+).*$/)[1];
Copy link
Owner

Choose a reason for hiding this comment

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

Does bintray not allow prerelease identifiers?

@avh4 avh4 changed the base branch from master to json March 26, 2017 04:01
@avh4 avh4 changed the base branch from json to master March 26, 2017 04:01
@avh4 avh4 changed the title WIP: Add npm installer Add npm installer Mar 26, 2017
@avh4 avh4 merged commit eb9cccd into master Mar 26, 2017
@avh4 avh4 deleted the npm branch March 26, 2017 17:45
@avh4
Copy link
Owner

avh4 commented Mar 26, 2017

(CI ❌ because the prs/branches were deleted before CI finished)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants