-
Notifications
You must be signed in to change notification settings - Fork 183
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
Move entire docgeneration into common tools #1219
Conversation
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
Co-authored-by: Wes Haggard <[email protected]>
console.log("Run PopulateList") | ||
|
||
$('h4').each(function () { | ||
var pkgName = $(this).text().replace(PACKAGE_REGEX, PACKAGE_REPLACEMENT) |
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.
Does this work correctly if the REGEX is empty?
Also I assume you added this to handle the JS case correct?
# Update main.js package regex and replacement | ||
$mainJsContent = $mainJsContent -replace "var PACKAGE_REGEX = ''", "var PACKAGE_REGEX = '$packageRegex'" | ||
$mainJsContent = $mainJsContent -replace "var PACKAGE_REPLACEMENT = ''", "var PACKAGE_REPLACEMENT = '$regexReplacement'" | ||
Set-Content -Path $MainJsPath -Value $mainJsContent |
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.
Given you used the -Raw
option in get-content you should use -NoNewLine
in the set-content.
# Update main.js var index html | ||
$mainJsContent = $mainJsContent -replace "var INDEX_HTML = ''", "var INDEX_HTML = '$indexhtmlloc'" | ||
# Update main.js package regex and replacement | ||
$mainJsContent = $mainJsContent -replace "var PACKAGE_REGEX = ''", "var PACKAGE_REGEX = $packageRegex" |
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.
Should $packageRegex
have quotes around it?
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.
JS actually read regex as only surrounded /.../
without quotes
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.
Tested in JS, will test the rest repo as well
populateOptions($('#navbar'), pkgName) | ||
} | ||
}) | ||
|
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.
nit: There are a lot of empty lines at the end of this file that can be removed.
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.
A couple questions/comments but otherwise looks good once you have finished your testing.
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Several features add-on:
Tested in Java