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

Automate rebuilds of dist/ when we land code on master #404

Closed
humphd opened this issue Sep 18, 2018 · 7 comments
Closed

Automate rebuilds of dist/ when we land code on master #404

humphd opened this issue Sep 18, 2018 · 7 comments
Assignees

Comments

@humphd
Copy link
Contributor

humphd commented Sep 18, 2018

We keep a pre-built version of Filer's bundle in dist/. Currently, it will lag behind the code on master unless manually updated. This is error prone, since people will forget, or not know to do this. We should consider letting Travis do it for us when we're merging onto master.

Similarly, once we have that, we should also tackle npm republishing of the module.

@0xazure
Copy link
Contributor

0xazure commented Sep 19, 2018

Is there a desire to maintain (and version) dist/ as part of the codebase, or would something like GitHub releases with possible uploading the built code from Travis CI achieve the same result?

@0xazure
Copy link
Contributor

0xazure commented Sep 19, 2018

Travis CI also appears to support automatically publishing to npmjs.org after a successful build (as well as a number of other providers).

@humphd
Copy link
Contributor Author

humphd commented Sep 19, 2018

@0xazure to your first question, I think that the least surprising thing would be for the code in dist/ to match the version you get when you npm install filer. As such, it should really get updated when we bump the package version (maybe on every commit, or maybe manually?).

To your second point, I agree that having travis do this via a deploy step on Travis is the best option.

@0xazure
Copy link
Contributor

0xazure commented Sep 21, 2018

I think that the least surprising thing would be for the code in dist/ to match the version you get when you npm install filer.

Definitely agreed that this is the least surprising thing. I wanted to confirm that we want to keep dist/ in the first place, or if replacing it with another distribution method (such as GitHub releases) was also an option.

I ask because I'm not as familiar with the practice of versioning build artifacts as part of the main project repository, I'm more used to seeing them published to a repository (such as npm) and/or available through a releases page (such as GitHub releases).

@humphd
Copy link
Contributor Author

humphd commented Sep 21, 2018

I would say that we can probably move to a model where we only release via npm. That would need some discussion and agreement. When we started the project, a lot of the current release mechanisms didn't exist.

  • Releases on GitHub are not common for js libs ime.
  • Having the build artifacts in the tree is nice, since you can just pull it in via a <script> tag in a browser (no install)
  • Having it in npm is obviously nice if you are doing a build step in your code.

@modeswitch what do you want to do?

@humphd
Copy link
Contributor Author

humphd commented Dec 4, 2018

Let's leverage npm and Travis CI for this, and deploy when we tag a release. Some background materials on how to do this:

Probably we can remove dist/ from the repo, and just have it be available as part of what gets pushed to npm. I'm not 100% sure on the conventions around doing this, so we should try to find some example projects already doing it, and understand best practices.

Also, we should figure out if there is a CDN we can/should update when we release something.

@humphd
Copy link
Contributor Author

humphd commented Dec 21, 2018

This is fixed now.

@humphd humphd closed this as completed Dec 21, 2018
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 a pull request may close this issue.

2 participants