-
Notifications
You must be signed in to change notification settings - Fork 97
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
Display GitHub Pages URL after ng deploy #107
Display GitHub Pages URL after ng deploy #107
Conversation
Messege to be shown is like
|
src/engine/engine.ts
Outdated
@@ -32,7 +32,7 @@ export async function run( | |||
await publishViaGhPages(ghpages, dir, options, logger); | |||
|
|||
logger.info( | |||
'🚀 Successfully published via angular-cli-ghpages! Have a nice day!' | |||
`🚀 Successfully published ${options.ghPagesUrl}via angular-cli-ghpages! Have a nice day!` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`🚀 Successfully published ${options.ghPagesUrl}via angular-cli-ghpages! Have a nice day!` | |
`🚀 Successfully published ${options.ghPagesUrl} via angular-cli-ghpages! Have a nice day!` |
src/engine/engine.ts
Outdated
@@ -278,3 +282,20 @@ async function getRemoteUrl(options) { | |||
const git = new Git(process.cwd(), options.git); | |||
return await git.getRemoteUrl(options.remote); | |||
} | |||
|
|||
function tryParseGhPagesUrl(options: any): any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options
and return value shouldn't be typeof any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Than you for the review.
Type of options
is actually too complex to define in a function.
Instead of adding tryParseGhPagesUrl, it is enough to parse url in prepareOptions
function.
What do you think?
if (options.repo) {
// Not assume custom domain page
let trimEndDotGit = options.repo
.replace(/\/\s*$/, '')
.replace(/\.git\s*$/, '');
const matchEndsWithRepoName = trimEndDotGit.match(/github.com(\/|:)(.*)\/(.*)$/);
if (matchEndsWithRepoName) {
options.ghPagesUrl = `https://${matchEndsWithRepoName[2]}.github.io/${matchEndsWithRepoName[3]}`;
}
}
return options;
@@ -10,5 +10,6 @@ export const defaults = { | |||
cname: undefined, | |||
dryRun: false, | |||
remote: 'origin', | |||
ghPagesUrl: '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably the prop should just be named url
? @JohannesHoppe / @fmalcher what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all: Thanks for the PR! It's a great start..🤗
Regarding this part:
I don't think this should be an option at all. There is no need to set this via options, it should be only shown in the output. Such a change would also make the overall PR smaller!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with your point: There is no need to set this via options
.
Anyway, I'll make another patch to apply the feedback.
This reverts commit 44532a4.
`tryParseGhPagesUrl` is short, thus I think that it is not a problem to include it into `prepareOptions`.
I am generating this pull request w.r.t this Issue #94.
In this PR, I have one limitation: I assume only 'github.io' pages. Custom domain page is not intended.
For custom domain page users, unexpected url would be displayed.