-
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
handle glob parsing in shell caommands #721
Conversation
The changes in the pull request primarily focus on the following areas:
No functional issues are found in the changes, and the code seems to be improved on clarity and accuracy of comments. There is no violation of principles of reliability, security, scalability, and performance. So, my response is "LGTM 🚀".
|
@@ -89,7 +89,7 @@ Options: | |||
-td, --test-delay <string> delay between tests in seconds | |||
--cache enable LLM result cache | |||
-v, --verbose verbose output | |||
-pv, --promptfoo-version [version] promptfoo version, default is 0.82.0 | |||
-pv, --promptfoo-version [version] promptfoo version, default is 0.89.3 |
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 flag --promptfoo-version
seems to be a typo; it should be --prompt-version
.
generated by pr-docs-review-commit
flag_typo
logError( | ||
"pull request review: no pull request information found" | ||
"pull request description: no pull request information found" |
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 error message has been changed from "pull request review: no pull request information found" to "pull request description: no pull request information found". Please ensure that this change is intentional and does not affect the debugging process. 🧐
generated by pr-review-commit
error_message_change
| "openai:gpt-3.5-turbo" | ||
| "ollama:phi3" | ||
| "ollama:llama3" | ||
| "ollama:mixtral" | ||
| 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 model options have been changed. The new options include "openai:gpt-4o" and "openai:gpt-4o-mini". Please ensure that these models are supported and the change is intentional. 🤔
generated by pr-review-commit
model_options_change
@@ -1439,7 +1439,7 @@ interface DefDataOptions | |||
extends Omit<ContextExpansionOptions, "maxTokens">, | |||
DataFilter { | |||
/** | |||
* Output format in the prompt. Defaults to markdownified CSV | |||
* Output format in the prompt. Defaults to Markdown table rendering. |
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 comment for the 'format' option in 'DefDataOptions' has been changed from "Output format in the prompt. Defaults to markdownified CSV" to "Output format in the prompt. Defaults to Markdown table rendering.". Please ensure that this change is accurate and does not mislead the users. 📝
generated by pr-review-commit
comment_change
packages/cli/src/run.ts
Outdated
} else { | ||
logError( | ||
"pull request description: no pull request information found" | ||
) |
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 error handling logic for the case when no pull request information is found has been changed. Previously, it was simply logging an error message. Now, it tries to query GitHub using the CLI and only logs an error if that also fails. This could potentially lead to unhandled exceptions if the GitHub CLI is not installed or not functioning correctly. It would be safer to wrap the new code in a try-catch block and handle any exceptions that might occur. 🛠️
generated by pr-review-commit
error_handling
logVerbose("github query failed") | ||
return undefined | ||
} | ||
} |
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 function githubQueryEnvUsingCli
is using the GitHub CLI to query for repository and issue information. This introduces a new dependency on the GitHub CLI being installed and correctly configured on the system where this code is running. If the GitHub CLI is not available, this function will fail. It would be better to handle this dependency explicitly and provide a clear error message if the GitHub CLI is not available. 📦
generated by pr-review-commit
cli_dependency
@@ -425,7 +425,8 @@ export async function runScript( | |||
} | |||
|
|||
if (pullRequestReviews && result.annotations?.length) { | |||
const info = githubParseEnv(process.env) | |||
// github action or repo |
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 function githubParseEnv
is now an async function, but it's not being awaited properly. This could lead to unexpected behavior.
generated by pr-review-commit
async_function
@@ -436,7 +437,8 @@ export async function runScript( | |||
} | |||
|
|||
if (pullRequestComment && result.text) { | |||
const info = githubParseEnv(process.env) | |||
// github action or repo | |||
const info = await githubParseEnv(process.env) |
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 function githubParseEnv
is now an async function, but it's not being awaited properly. This could lead to unexpected behavior.
generated by pr-review-commit
async_function
…ection info usage
In the hush of twilight's glow,
|
This pull request includes updates to the model options, modifications to binary files across multiple paths, and fixes for typos in the prompt_template.d.ts file. The changes in the model options include adding new entries in the settings.json file and fixing typos in the prompt_template.d.ts file. The modifications to binary files are reflected in the git patches provided. Overall, this pull request aims to improve the codebase by updating model options, modifying binary files, and fixing typos.
package.json
files have been upgraded.githubParseEnv
has been expanded to include async logic to fetch missing repo, owner, and issue data.githubQueryEnvUsingCli
has been introduced but not used.runScript
function has been updated to asynchronously parse Github environment data.githubUpdatePullRequestDescription
,githubCreatePullRequestReview
, andgithubCreateIssueComment
have been adjusted to assert the presence of certain variables.azureDevOpsParseEnv
function inpackages/core/src/azuredevops.ts
has been converted to an async function.defaultBranch
to thepr-describe
script in the sample packages. This denotes the default branch of the repository.packages/core/src/types/prompt_template.d.ts
.model
option inModelConnectionOptions
now accepts anOptionsOrString
type allowing the usage of custom strings.🔧 Dependency Updates: Updated versions for several dependencies across multiple
package.json
files, such as@astrojs/starlight
,gpt-tokenizer
, andpromptfoo
.🚀 Async Enhancements: Converted several functions to asynchronous, including
githubParseEnv
andazureDevOpsParseEnv
, to likely improve performance and handle asynchronous operations.🐛 Bug Fixes and Improvements: Added checks and improved logic for GitHub and Azure DevOps interactions, including better handling of repository and issue information.
📜 Documentation and Typo Corrections: Fixed typos and improved documentation comments, ensuring clarity and correctness.
🔄 Command Refactoring: Introduced reusable command variables for Git operations in scripts, enhancing maintainability and readability.
🛠️ Code Quality: Made code style improvements, such as ensuring proper assertions and formatting adjustments, to enhance code quality and consistency.