Skip to content
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

Release v0.2.4 #50

Merged
merged 10 commits into from
Aug 2, 2024
Merged

Release v0.2.4 #50

merged 10 commits into from
Aug 2, 2024

Conversation

jcrodriguez1989
Copy link
Owner

No description provided.

Copy link

github-actions bot commented Nov 9, 2023

Here is the code review for the GitHub PR:

  1. In the file DESCRIPTION, on line 5, the version should be updated from 0.2.3 to 0.2.4.

  2. In the file R/addins.R, on line 39, the condition should be changed from as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)) to get_verbosity().

  3. In the file R/addins.R, on line 52, the condition should be changed from as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)) to get_verbosity().

  4. In the file R/chatgpt-package.R, on line 14, the variable api_url should be moved to the top of the file and imported using the get_verbosity() function.

  5. In the file R/get_verbosity.R, on line 4, the function get_verbosity() should be added.

  6. In the file R/gpt_get_completions.R, on line 26, the condition should be changed from as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)) to get_verbosity().

  7. In the file R/gpt_get_completions.R, on line 33, the URL should be constructed using the api_url variable: paste0(api_url, "/chat/completions").

  8. In the file R/parse_response.R, on line 6, the function parse_response() should accept an additional argument verbosity = get_verbosity().

  9. In the file R/parse_response.R, on line 8, if the verbosity level is greater than 1, the JSON response should be printed using message(toJSON(response, pretty = TRUE)).

  10. In the file R/parse_response.R, on line 15, the function parse_response() should be modified to use the verbosity argument while calling the parse_response() function.

  11. In the file README.Rmd, on line 147, a new section should be added to explain how to switch the OPENAI's API URL using the OPENAI_API_URL environment variable.

  12. In the file README.md, on line 351, the same new section should be added to explain how to switch the OPENAI's API URL using the OPENAI_API_URL environment variable.

  13. In the file `man/get_The response seems incomplete. Could you please provide the remaining part?_

Copy link

github-actions bot commented Nov 9, 2023

I have reviewed the code and found the following issues:

  1. In the file .github/workflows/code-review-gpt.yaml on line 35, the diff command should be updated from GITHUB_HEAD_REF to GITHUB_BASE_REF. It should be diff <- system(paste("git diff", GITHUB_BASE_REF, GITHUB_HEAD_REF), intern = TRUE).

  2. In the file DESCRIPTION on line 4, the version should be updated from 0.2.4 to 0.2.3.

  3. In the file R/addins.R on line 37, the condition should be updated from get_verbosity() to as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)).

  4. In the file R/addins.R on line 78, the condition should be updated from get_verbosity() to as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)).

  5. In the file R/gpt_get_completions.R on line 23, the condition should be updated from get_verbosity() to as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)).

  6. In the file `R/parse_response.R(continued)

  7. In the file R/parse_response.R on line 3, the verbosity parameter is no longer used and can be removed from the function signature.

  8. In the file R/parse_response.R on line 6, the verbosity > 1 condition can be removed since the verbosity parameter has been removed.

  9. In the file README.Rmd on line 145, the section "Switch OPENAI's API URL" can be removed since the functionality is no longer present in the code.

  10. In the file README.md on line 349, the section "Switch OPENAI's API URL" can be removed since the functionality is no longer present in the code.

  11. The file man/get_verbosity.Rd can be removed since the get_verbosity function has been deleted.

  12. In the file man/parse_response.Rd, the documentation should be updated to remove the verbosity parameter from the function description.

Please review these issues and make the necessary changes. Let me know if you need any further assistance.

Copy link

github-actions bot commented Nov 9, 2023

Code Review:

  1. In .github/workflows/code-review-gpt.yaml, line 38:

    • The git diff command should be modified to include double quotes around GITHUB_BASE_REF and GITHUB_HEAD_REF variables to handle cases where the branch names contain special characters or spaces.
    • Replace paste with paste0 for concatenation to avoid unnecessary whitespace.
    • Consider adding error handling in case the git fetch origin or git diff commands fail.
  2. In DESCRIPTION, line 8:

    • The version number should be updated to 0.2.4.
  3. In R/addins.R, line 39:

    • Instead of using as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)), consider creating a separate function get_verbosity() (as added in this PR) to handle verbosity checks and make the code more readable.
    • Replace message with cat for consistency with other parts of the code.
  4. In R/addins.R, line 78:

    • Instead of using as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)), use get_verbosity() for verbosity checks.
    • Replace messageLine 82: Replace updateTextAreaInputwithupdateTextInput` to update the input element correctly.
  5. In R/chatgpt-package.R, line 14:

    • The api_url variable is defined using the Sys.getenv function with a default value of "https://api.openai.com/v1". It is recommended to use the default value directly instead of using Sys.getenv.
  6. In R/get_verbosity.R, line 5:

    • Add proper documentation using roxygen style comments to describe the purpose and usage of the function.
  7. In R/gpt_get_completions.R, line 48:

    • Instead of hardcoding the API URL as "https://api.openai.com/v1/chat/completions", use the api_url variable defined in R/chatgpt-package.R.
  8. In R/gpt_get_completions.R, line 55:

    • Use paste0(api_url, "/chat/completions") instead of directly using "https://api.openai.com/v1/chat/completions".
  9. In R/parse_response.R, line 14:

    • Add a parameter verbosity with a defaultvalue of get_verbosity() to control the verbosity level of the function.
  10. In R/parse_response.R, line 15:

    • If verbosity is greater than 1, use lapply and message to print the raw JSON response for each element in raw_responses with pretty formatting.
  11. In README.Rmd and README.md, lines 144-149:

    • Add a section explaining how to switch the OPENAI_API_URL by setting the OPENAI_API_URL environment variable.
  12. In README.Rmd and README.md, lines 342-347:

    • In the example for setting the OPENAI_PROXY environment variable, use OPENAI_API_URL instead of OPENAI_PROXY.

Those are the issues found in the code review.

Copy link

github-actions bot commented Nov 9, 2023

Code Review Findings:

  1. In the file .github/workflows/code-review-gpt.yaml, line 36, the prompt message could be clearer. Consider providing more information about the PR, such as the commit message or the author.

  2. In the file DESCRIPTION, line 4, the version number seems to be incorrect. It is labeled as 0.2.3, but it should be 0.2.4 to match the current version.

  3. In the file R/addins.R, line 37, the condition as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)) can be simplified to just as.logical(Sys.getenv("OPENAI_VERBOSE")). The default value of TRUE is not needed as this already captures the logic.

  4. In the file R/addins.R, lines 45 and 49, the code is checking the verbosity level twice. It can be simplified by removing the second check and directly passing chatgpt_reply to the message function.

  5. In the file R/gpt_get_completions.R, line 25, the condition as.logical(Sys.getenv("OPENAI_VERBOSE", TRUE)) can be simplified to just as.logical(SSys.getenv("OPENAI_VERBOSE")). The default value of TRUE is not needed as this already captures the logic.

  6. In the file R/parse_response.R, the verbosity parameter is not used in the function. Consider removing it if it is not necessary.

  7. In the file README.Rmd, lines 141-144, there is a section titled "Switch OPENAI's API URL" that provides instructions for replacing the default API URL. However, the example URL provided (https://api.chatanywhere.com.cn) seems incorrect or incomplete. Please verify if this is intentional.

Overall, the code changes in the PR seem reasonable and there are only a few minor issues found. Please review the above findings and make any necessary adjustments.

@jcrodriguez1989 jcrodriguez1989 merged commit d22a39a into main Aug 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant