feat(deploy): add custom-domain support for gh-pages deployment (#1781) #3392
Conversation
austingreendev
changed the title
|
@filipesilva could you review this please? |
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
|
The CLA is good, we just merged the master branch. |
|
Seems I cannot add reviewers to this PR, not sure why. |
|
@Austin94 it is true we don't have tests for The current branch should have no changes and that makes it hard to make an e2e test for it. The current test mostly mocks exec calls. Adding a test to verify these two calls would require a way to interrupt (and resume) the command, which would require a lot of work. This is also a feature I want to re-do from scratch in the coming months as it's very complex and hard to test currently. So personally I'm fine with not having these two function calls not tested and letting the PR in, as the changes are not breaking. @hansl if you disagree let me know. |
|
@filipesilva that makes perfect sense to me. Thanks for taking the time to explain the reasoning behind it. |
| @@ -76,6 +78,11 @@ const githubPagesDeployCommand = Command.extend({ | |||
| type: String, | |||
| default: null, | |||
| aliases: ['bh'] | |||
| }, { | |||
| name: 'custom-domain', | |||
There was a problem hiding this comment.
Can we have an alias cd for this?
|
CLAs look good, thanks! |
|
I'll merge this when the PR is rebased and the conflicts resolved. |
|
My apologies. For some reason I thought this was already merged!
I will rebase/merge this evening.
…On Jan 12, 2017 1:13 PM, "Hans" ***@***.***> wrote:
I'll merge this when the PR is rebased and the conflicts resolved.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3392 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AD1_qZvrEG3Ef0ylHWTq8HPPSUAXIUkeks5rRntWgaJpZM4LDy4H>
.
|
Creates CNAME file if --custom-domain flag is provided. addresses (#1781)
|
Ok, everything should be playing nicely now. The addition of the |
|
@Austin94 cheers for adding this! |
…lar#1781) (angular#3392) Creates CNAME file if --custom-domain flag is provided. addresses (angular#1781)
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
feature for (#1781)
This PR adds the new
--custom-domainflag to thegithub-pages:deploycommand to create a CNAME file to support Github Pages custom domains.I am looking for some guidance on the testing for this since
github-pages-deploy.spec.jsdoesn't contain any logic/acceptance criteria to ensure that certain files are created. It is only checkinggit related commands and http requests.
Currently the similar method
createNotFoundPage()is not tested either. Either way, all tests and linting pass. I also have some spare domains and tested that Github's DNS picks the CNAME up with a new branch correctly.