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

Travis GitHub Releases #2550

Merged
merged 2 commits into from
May 14, 2016
Merged

Travis GitHub Releases #2550

merged 2 commits into from
May 14, 2016

Conversation

simonbrunel
Copy link
Member

Update .travis.yml to build and upload dist/* files when a tag is created, then remove dist/* files from the repository. The new release process would be:

  • draft a release (optional)
  • create the associated git tag
  • a build is triggered by Travis for this tag (automatic)
  • generated dist/* files are uploaded along the release (automatic)

We will also need to update CDNjs to use these uploaded files.

Proof of concept: https://github.com/chartjs/chartjs-color/releases/tag/2.0.0

Travis now builds and deploys these files for all tags, so we don't need to have them on the repository anymore.
@panzarino
Copy link
Contributor

panzarino commented May 14, 2016

I think we should keep the dist files in the repo. When people come here from the website for the first time they probably click the download button, without checking for a release first (at least that's what I did a while ago when I started using Chart.js). (Plus doesn't this make private keys available to everyone in travis.yml?) Either way will be fine I'm sure, just my 2¢

Edit: After some more research, this probably is a better and cleaner system, I think it would be good

@tannerlinsley
Copy link
Contributor

Very cool man!

@etimberg
Copy link
Member

@zachpanz88 the token is encrypted from the actual token. The alternate is that we add it as a environment variable in travis. I don't know which is better @simonbrunel

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.577% when pulling 2d9bf77 on simonbrunel:travis-dist into b64e03d on chartjs:master.

@simonbrunel
Copy link
Member Author

simonbrunel commented May 14, 2016

The whole thing here is to remove dist files from the repo, it doesn't make sense to have them here:

  • never synced with sources, people should download all the sources to only use built files
  • seen some (many?) PR which commit these files, then revert the change
  • annoying when rebasing stuff (need to stash or revert these files)

For first time users, we should had a "download" link (readme.md and website?) to the releases page.

I encrypted the token with the Travis command line, meaning it uses a public key for encryption, decryption can only be done with a private key owned by Travis (if I understood correctly). So I would have no worries about the API key.

@panzarino
Copy link
Contributor

Sounds good to me. I do think that we should add a link on the website to the downloads page in addition.

@simonbrunel
Copy link
Member Author

simonbrunel commented May 14, 2016

Just found this link: https://github.com/chartjs/Chart.js/releases/latest

We could use it in the website, README.md and the documentation.

@simonbrunel simonbrunel merged commit 2d9bf77 into chartjs:master May 14, 2016
@simonbrunel
Copy link
Member Author

PR incidentally merged (sorry for that), master has been reset to the correct commit.

I created this new PR #2552.

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.

5 participants