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

chore: Pull gh-pages building to deploy script #445

Merged
merged 1 commit into from Aug 15, 2018

Conversation

nschonni
Copy link
Contributor

Move the logic for doing the manual gh-pages deployment to the built-in
Travis-CI “script” deployment.
This allows all branches to be built and tested, but only the master
branch to be deployed.


This also allows people to setup and test their own gh-pages since it used the fork's repo slug as long as they have setup their own GH_TOKEN value.
EX: when I put the filter back to the master branch, the deployment no-ops https://travis-ci.org/nschonni/aria-practices/builds/269029973
EX: when I had it set to the feature branch for testing, the deployment goes ahead https://travis-ci.org/nschonni/aria-practices/builds/269029438

Closes #433

@mcking65
Copy link
Contributor

@nschonni, thanks. I think this could prove helpful.

Before I can test it out, I'll have to switch to using a token for auth. And, we should make sure all the editors are using tokens before we merge as well.

@nschonni
Copy link
Contributor Author

If they don't have the GH_TOKEN value set, it just skips the deployment on their forks. When the branches are in this repository, it will use the token that @michael-n-cooper has already setup.

I added this comment in the .travis.yml, but I could go back to using an echo with the instructions. Or just add it to the contributing/README.

# GH_TOKEN shoulb be defined by following https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings

Copy link
Contributor

@jessebeach jessebeach left a comment

Choose a reason for hiding this comment

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

I trust that you tested this :)

@nschonni
Copy link
Contributor Author

@jessebeach thanks for pinging me on this. I rebased again for the conflicts, and the only "issue" this has is that people forking don't get warned about needing to set their own GitHub token on Travis if they want their own GH Pages to build. This was kind of the situation before since PR builds don't have access to the secure variables now either

@nschonni nschonni mentioned this pull request Jul 30, 2018
@mcking65
Copy link
Contributor

mcking65 commented Aug 2, 2018

@michael-n-cooper, we really need the ability to run checks on PRs with a base branch other than master. You previously voiced a concern regarding history in the gh-pages branch. So far, in my experience, I see running checks on branches other than master as much higher value than that history; I never look at logs for gh-pages and am not sure I would ever have a reason to do so.

Are you OK with this change, especially in light of our recent discussion to build the apg-1.2 branch in a 1.2 subdir in the gh-pages branch?

@nschonni
Copy link
Contributor Author

nschonni commented Aug 2, 2018

This should still keep the history from gh-pages. I think it might have been another one where I did a bare branch push that would lose the history

@jessebeach
Copy link
Contributor

I rebased again for the conflicts, and the only "issue" this has is that people forking don't get warned about needing to set their own GitHub token on Travis if they want their own GH Pages to build.

@nschonni could you move the conditional from the .travis.yml file to an if/else in the travis-deploy.sh script that echoes a warning in the CLI if GH_TOKEN isn't set?

if [ $GH_TOKEN != '' ]; then
  # run the deploy
else
  # warn the user
fi

@nschonni nschonni force-pushed the test-all-branches branch 2 times, most recently from 6d575d9 to d67e56a Compare August 6, 2018 02:50
Move the logic for doing the manual gh-pages deployment to the built-in
Travis-CI “script” deployment.
This allows all branches to be built and tested, but only the master
branch to be deployed.
@nschonni
Copy link
Contributor Author

nschonni commented Aug 6, 2018

@jessebeach done, used a zero check and an exit 1 so it should be visible that the deployment failed because of the missing token. I haven't really tested this in awhile so it could probably use someone else testing

@jessebeach
Copy link
Contributor

LGTM

@mcking65
Copy link
Contributor

OK ... I've spent enough time just looking at this ... I'm going to merge so I can test for real.

@mcking65 mcking65 merged commit 458beba into w3c:master Aug 15, 2018
michael-n-cooper pushed a commit that referenced this pull request Aug 15, 2018
chore: Pull gh-pages building to deploy script (pull #445)

Move the logic for doing the manual gh-pages deployment to the built-in
Travis-CI “script” deployment.
This allows all branches to be built and tested, but only the master
branch to be deployed.
mcking65 added a commit that referenced this pull request Aug 15, 2018
Trivial editorial change to test new build/test/deploy scripts from PR #445.
michael-n-cooper pushed a commit that referenced this pull request Aug 15, 2018
Treegrid Pattern: Change new pattern warning to editor's note

Trivial editorial change to test new build/test/deploy scripts from PR #445.
@mcking65
Copy link
Contributor

so far, so good... first build to gh-pages by pushing to master passed. The logs for the build look clean.

@nschonni nschonni deleted the test-all-branches branch August 15, 2018 01:01
@nschonni
Copy link
Contributor Author

@mcking65 if you close and open the existing PRs I think they should get picked up by the build now

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.

None yet

3 participants