-
Notifications
You must be signed in to change notification settings - Fork 126
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
github action documentation #505
Conversation
@@ -27,7 +27,7 @@ export async function resolveFileContent( | |||
const { trace } = options || {} | |||
const { filename } = file | |||
if (file.content) return file | |||
|
|||
if (!filename) return file |
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.
The variable adapter
is declared but never used within the scope of the HTTPS_REGEX.test(filename)
block. This could be an oversight or leftover from a refactor. Consider removing it if it's not needed, or ensure it's being used appropriately if it was intended for some functionality. 🤔
generated by genaiscript pr-review-commit
"-prc, --pull-request-comment <string>", | ||
"create comment on a pull request." | ||
"-prc, --pull-request-comment [string]", | ||
"create comment on a pull request.", "" | ||
) |
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.
Similar to the --pull-request-comment
option, the --pull-request-description
option also has an unnecessary default value ""
. It would be cleaner to have --pull-request-description [string]
without the default value. Keep it up! 👍
generated by genaiscript pr-review-commit
@@ -101,12 +101,12 @@ export async function cli() { | |||
) | |||
.option("-ocl, --out-changelog <string>", "output file for changelogs") | |||
.option( | |||
"-prc, --pull-request-comment <string>", | |||
"create comment on a pull request." | |||
"-prc, --pull-request-comment [string]", |
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.
The option for pull-request-description should not have a default value as it is now optional, indicated by the square brackets. Remove the default empty string to avoid confusion.
generated by genaiscript pr-review-commit
|
"-prc, --pull-request-comment <string>", | ||
"create comment on a pull request." | ||
"-prc, --pull-request-comment [string]", | ||
"create comment on a pull request.", "" | ||
) |
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.
Similarly, the square brackets around [string]
for the --pull-request-description
option indicate that the argument is optional, but there's no default value provided in the code. Make sure to provide a default value or handle the case where the argument might be omitted. 👍
generated by genaiscript pr-review-commit
Caution The square brackets around Caution Similarly, the square brackets around Caution The check
|
The provided
Overall, the changes look good and aim to improve the user experience and integration with GitHub workflows. LGTM 🚀 No functional issues are apparent from the diff, and the TypeScript compiler should handle type checking. The updates seem to be well-aligned with best practices and do not introduce any obvious regressions or concerns.
|
Document various new flags to support github actions
Caution
The square brackets around
[string]
for the--pull-request-comment
option suggest that the argument is optional, but the default value""
is not provided in the code. Consider adding a default value or handling the case where the argument might be omitted. 😊 (packages/cli/src/cli.ts#L101)Caution
Similarly, the square brackets around
[string]
for the--pull-request-description
option indicate that the argument is optional, but there's no default value provided in the code. Make sure to provide a default value or handle the case where the argument might be omitted. 👍 (packages/cli/src/cli.ts#L107)Caution
The check
if (!filename) return file
is added without any comment or explanation. It's important to document the reason for such a check to avoid confusion for future maintainers of the code. Consider adding a comment to explain why the filename check is necessary. 📝 (packages/core/src/file.ts#L27)pr-describe.genai.js
for summarizing changes in a pull request.pr-review-commit.genai.js
for reviewing the latest commit in a pull request.promptfoo
dependency from^0.60.0
to^0.61.0
in CLI package.resolveFileContent
function by adding a check for the existence offilename
before proceeding.The provided
GIT_DIFF
indicates several updates and additions across documentation and code files. Here's a high-level review:automating-scripts.mdx
enhance clarity on using GenAIScript with CI/CD pipelines, specifically GitHub Actions, including caching, annotations, and pull request integration.commands.md
file shows updates to CLI options, improving user experience by clarifying the use of flags for pull request comments and descriptions.index.mdx
file reflects a minor update, linking the CLI usage to automation scenarios.annotations.md
file now includes a section on GitHub Pull Request Review Comments, which is a valuable addition for integrating annotations directly into code reviews.package.json
files suggest maintenance and keeping up with the latest versions, which is good for security and performance.cli.ts
andrun.ts
) seem to handle pull request comments and descriptions more robustly, with default values ensuring smoother operations.Overall, the changes look good and aim to improve the user experience and integration with GitHub workflows. LGTM 🚀
No functional issues are apparent from the diff, and the TypeScript compiler should handle type checking. The updates seem to be well-aligned with best practices and do not introduce any obvious regressions or concerns.