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

CI: Add automatic Github Pages deployment #143

Merged
merged 1 commit into from
Dec 5, 2017
Merged

CI: Add automatic Github Pages deployment #143

merged 1 commit into from
Dec 5, 2017

Conversation

nalinbhardwaj
Copy link
Member

@nalinbhardwaj nalinbhardwaj commented Dec 3, 2017

Automates Github Pages deployment by making travis commit any changes to master to gh-pages branch, and other branch changes to gh-pages-branch/"original branch name"

Currently, you can see it works on my fork. For example, commits to master automatically get pushed to gh-pages and deployed on my website http://nibnalin.me/coala-html/#/ . Branch CI-testing gets built and pushed to gh-pages-branch/CI-testing. Note that the original issue mentions using gh-pages/<branch name> but that couldn't be done since git would error out due to existence of gh-pages branch.

data folder used for building the website has to of course now be moved to master(I've done that), and the failing tests are unrelated to my PR, and have already been verified to fail on clean master by @Mixih

TODO: I would require an owner to provide a deploy key.

Closes #133

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

  • I read the commit guidelines and I've followed
    them.
  • I ran coala over my code locally. (All commits have to pass
    individually.
    It is not sufficient to have "fixup commits" on your PR,
    our bot will still report the issues for the previous commit.) You will
    likely receive a lot of bot comments and build failures if coala does not
    pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

Copy link
Member

@yukiisbored yukiisbored left a comment

Choose a reason for hiding this comment

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

A couple of problems.

  1. Please use Travis-CI GitHub pages deployment feature: https://docs.travis-ci.com/user/deployment/pages/
  2. Your diff is so huge for such a simple thing, please remove the unnecessary stuff.

@yukiisbored
Copy link
Member

deack 454fe03

@nalinbhardwaj
Copy link
Member Author

nalinbhardwaj commented Dec 3, 2017

@yukiisbored Can you elaborate on the 2nd point? Most of the diff comes from putting the ‘data’ folder from gh-pages branch in master. I’ve only modified 2 files, .travis.yml and .ci/deploy.sh .
The reason behind putting data here is to allow easy change of data to use to build gh-pages, otherwise you would earlier have to rebuild it manually on changing data.
Regarding 1, I’ll check that out but currently coala/community seems to have a similar deployment system. Should that be changed as well? Also, I think I read online that the way I(and community repo) do it is more secure/modular?

@yukiisbored
Copy link
Member

yukiisbored commented Dec 3, 2017

@nalinbhardwaj About coala/community, no idea, @jayvdb probably didn't know about Travis CI GitHub Pages deployment feature. Anyway about the Data folder, didn't notice those are coala json output since I reviewed from my phone. But, can you confirm deployment go smoothly since it looks like the data folder isn't inside the public/ folder which won't be part of the deployed site.

@nalinbhardwaj
Copy link
Member Author

Yep @yukiisbored deployment is currently smooth and fine with data where it is. If you want however I can move it to some other directory?

Also there’s a problem with the Travis Github pages building, since it’s a yaml file, I don’t think we can do stuff like pushing to custom branch(as I require for this issue) like master pushes to gh-pages but any other branch “xyz” pushes to gh-pages-branch/xyz ?

@yukiisbored
Copy link
Member

@yukiisbored
Copy link
Member

Also, If you want to see an example: https://github.com/yukiisbored/blog-archive/blob/blog/.travis.yml

@yukiisbored yukiisbored dismissed their stale review December 3, 2017 14:51

Asked other maintainers, seems like we prefer ssh deploy keys.

@yukiisbored
Copy link
Member

@nalinbhardwaj just asked @jayvdb, it seems SSH deploy keys are preferred so I'm gonna merge this, but I need to setup the deploy keys.

.travis.yml Outdated
@@ -32,6 +39,8 @@ script:
- python -m pytest --cov
- codecov
- docker run --volume=$(pwd):/app --workdir=/app coala/base coala-ci
- python3 setup.py install
- bash ./.ci/deploy.sh
Copy link
Member

Choose a reason for hiding this comment

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

This should be done on deploy phase so failing builds doesn't get deployed.

ssh-add ../deploy_key

# Now that we're all set up, we can push.
git push $SSH_REPO $TARGET_BRANCH
Copy link
Member

Choose a reason for hiding this comment

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

No EOL at EOF (copying ... :P)

@yukiisbored
Copy link
Member

Marking this as blocked since the builds are failing.

@@ -0,0 +1,7 @@
{
"data":"data",
Copy link
Member

Choose a reason for hiding this comment

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

please add JSONBear to .coafile

@@ -0,0 +1 @@
/home/tushar/coala
Copy link
Member

Choose a reason for hiding this comment

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

EOL at EOF

.travis.yml Outdated
@@ -1,12 +1,19 @@
sudo: required
dist: trusty

env:
global:
- ENCRYPTION_LABEL: "79272d66a36b"
Copy link
Member

Choose a reason for hiding this comment

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

Yaml lint bear pls

.travis.yml Outdated
@@ -32,6 +39,8 @@ script:
- python -m pytest --cov
- codecov
- docker run --volume=$(pwd):/app --workdir=/app coala/base coala-ci
- python3 setup.py install
- bash ./.ci/deploy.sh
Copy link
Member

Choose a reason for hiding this comment

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

this should be in deploy:

Copy link
Member Author

Choose a reason for hiding this comment

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

That’s a workaround for now, I’ll do that once tests start passing(as noted in PR description).

Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

A few things to fix

@nalinbhardwaj
Copy link
Member Author

For future readers, some discussion regarding the decisions can be found in Zulip.

.travis.yml Outdated
language: python
node_js:
- "4.1"
- "4.1"
Copy link
Member

Choose a reason for hiding this comment

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

Hey this is not needed, it just makes some noise on the diff.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooh, I just ran YAMLLintBear and applied patch to it without checking anything. Will fix everything.

.travis.yml Outdated
python:
- "3.4"
- "3.4"
Copy link
Member

Choose a reason for hiding this comment

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

Hey this is not needed, it just makes some noise on the diff.

.travis.yml Outdated
directories:
- node_modules
- coalahtml/_coalahtml/bower_components
pip: true
Copy link
Member

Choose a reason for hiding this comment

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

Hey this is not needed, it just makes some noise on the diff.

.travis.yml Outdated
- pip install colorama==0.3.7
- pip install -r requirements.txt
- pip install -r test-requirements.txt
- npm install - g bower
Copy link
Member

Choose a reason for hiding this comment

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

Hey this is not needed, it just makes some noise on the diff.
This breaks CI as well due to that space between - and g

.travis.yml Outdated
- cd -
# colorama needs to be upgraded to fix error when installed colorama
# version and required are different.
- pip install colorama == 0.3.7
Copy link
Member

Choose a reason for hiding this comment

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

Breaks CI, due to that space between packaage and version

.travis.yml Outdated
# colorama needs to be upgraded to fix error when installed colorama
# version and required are different.
- pip install colorama == 0.3.7
- pip install - r requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Break CI due to that space between the flag and the indicator

.travis.yml Outdated
# version and required are different.
- pip install colorama == 0.3.7
- pip install - r requirements.txt
- pip install - r test-requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Break CI due to that space between the flag and the indicator

.travis.yml Outdated

notifications:
email: false
email: false
Copy link
Member

Choose a reason for hiding this comment

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

Noise, not needed

.travis.yml Outdated
- python -m pytest --cov
- codecov
- docker run --volume=$(pwd):/app --workdir=/app coala/base coala-ci
- npm test
Copy link
Member

Choose a reason for hiding this comment

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

Not needed.

@yukiisbored
Copy link
Member

Sadly we can't merge it, master is broken :(

Copy link
Member

@blazeu blazeu left a comment

Choose a reason for hiding this comment

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

BTW, is .rultor.yml modified because you run YAMLLintBear ?

.ci/deploy.sh Outdated
exit 0
fi

# Commits to other branches should deploy to their respective gh-pages/<name>
Copy link
Member

Choose a reason for hiding this comment

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

Should be gh-pages-branch/<name> ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, typo.

.travis.yml Outdated
@@ -31,7 +36,11 @@ script:
- npm test
- python -m pytest --cov
- codecov
- docker run --volume=$(pwd):/app --workdir=/app coala/base coala-ci
- docker run --volume =$(pwd): / app --workdir = /app coala/base coala-ci
Copy link
Member

Choose a reason for hiding this comment

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

I think you shouldn't change this?
See https://travis-ci.org/coala/coala-html/builds/311194465#L686

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Just noticed the command is parsed like YAML

.rultor.yml Outdated
fast-forward: only
script:
- bash .ci/check_maintainership.sh
fast-forward: only
Copy link
Member

Choose a reason for hiding this comment

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

Please undo this.

.rultor.yml Outdated

decrypt:
rultor_github_secrets.sh: "repo/.ci/rultor_github_secrets.sh.asc"
rultor_github_secrets.sh: "repo/.ci/rultor_github_secrets.sh.asc"
Copy link
Member

Choose a reason for hiding this comment

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

This as well.

Automates Github Pages deployment by making travis commit any changes to master to gh-pages branch, and other branch changes to gh-pages-branch/<original branch name>.

Closes #133
@yukiisbored
Copy link
Member

ack 70dfa00

@nalinbhardwaj
Copy link
Member Author

Umm, @yukiisbored I think you forgot about the TODO from my PR description? That encrypted deploy key won’t work for this repo?

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

Successfully merging this pull request may close these issues.

Rebuild gh-pages branch on each push
5 participants