-
Notifications
You must be signed in to change notification settings - Fork 5
fix debug logs & 404 on docs after new deploy #156
Conversation
mordamax
commented
Jan 25, 2023
- Fix for debug logs + make help clearer
- fix 404 docs after redeployment
Fix for debug logs + make help clearer
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.
yarn-error.log
Outdated
@@ -0,0 +1,7776 @@ | |||
Arguments: |
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 guess this was an accidental commit.
Can you please delete this and add it to the .gitignore
file please?
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.
good catch, thanks!
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.
Looks good to me (except for the already mentioned error log)
@@ -54,7 +56,7 @@ export async function fetchCommandsConfiguration( | |||
} | |||
|
|||
export function getDocsUrl(commitHash: string): string { | |||
return new URL(path.join(config.cmdBotUrl, DOCS_PATH, getDocsFilename(commitHash))).toString() | |||
return new URL(path.join(config.cmdBotUrl, DOCS_URL_PATH, getDocsFilename(commitHash))).toString() |
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.
Using path.join
in URLs is probably a mistake, as the purpose of path.join
is to generate OS-specific slashes, while URLs always use /
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.
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.
thanks, i'd keep it for now
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.
LGTM, aside from yarn-error.log
, which was already mentioned