fix: testing improvements #102
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily refactors the documentation by moving developer-specific instructions from the main README.md to docs/DEVELOPING.md, which improves clarity for end-users. Additionally, it enhances the robustness of the check_entitlements.sh script by adding error handling for token acquisition and adopting shell scripting best practices. My review includes a suggestion to make the error handling in the shell script more concise and idiomatic. Overall, these are good improvements to the repository.
| if [ -z "$BEARER" ]; then | ||
| echo "❌ Failed to get access token." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This check is great for robustness! To make it more concise and idiomatic to shell scripting, you can use a short-circuit || operator. This also provides an opportunity to redirect the error message to stderr (>&2), which is a best practice for error reporting.
| if [ -z "$BEARER" ]; then | |
| echo "❌ Failed to get access token." | |
| exit 1 | |
| fi | |
| [ -n "$BEARER" ] || { echo "❌ Failed to get access token." >&2; exit 1; } |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements across the repository. The documentation has been refactored for better clarity by moving developer-specific information into its own file. The Python SDK's API has been simplified by consolidating two decryption methods into one, making it more intuitive to use. Additionally, a shell script has been made more robust. My feedback focuses on further enhancing the error handling in this script.
| if [ -z "$BEARER" ]; then | ||
| echo "❌ Failed to get access token." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The check for an empty BEARER token is a good addition. However, it doesn't account for the case where the curl command succeeds but the JSON response from the token endpoint does not contain an access_token. In that scenario, jq -r '.access_token' will output the string "null", causing the BEARER variable to be assigned this string. The current check [ -z "$BEARER" ] would pass, and the script would proceed with an invalid token.
To make this check more robust, you should also test if $BEARER is equal to the string "null".
| if [ -z "$BEARER" ]; then | |
| echo "❌ Failed to get access token." | |
| exit 1 | |
| fi | |
| if [ -z "$BEARER" ] || [ "$BEARER" == "null" ]; then | |
| echo "❌ Failed to get access token." | |
| exit 1 | |
| fi |
No description provided.