-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: read content of generate request #14113
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
Conversation
89b9518 to
7b60921
Compare
7b60921 to
fbb164e
Compare
.librarian/generate-request.json
Outdated
| @@ -0,0 +1,10 @@ | |||
| { | |||
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.
Real generation requests will go in this path, correct? So this actual file is just test data? If so; I suggest
- place this file under some sort of test directory (call it
$LIBRARIAN_TEST_DIRin this discussion) - configure the CLI to set the variable
$LIBRARIANto the value of the environment variable$LIBRARIAN_DIRif it exists, or otherwise the default you already have - when you run
test_handle_generate, set$LIBRARIAN_DIRto have the value of$LIBRARIAN_TEST_DIRso it reads the input from the test dir.
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.
I've addressed this comment. However, my initial intention was to remove this file once we complete the work since we don't need it (i.e. we only need it for local testing). We won't be testing bazelisk commands in our unit tests. Let me know what you think.
vchudnov-g
left a comment
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.
Looks good. Thanks for your changes.
| Returns: | ||
| dict: The parsed JSON content. | ||
| Raises: |
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 exceptions specify the file path? Cool.
Closes #14122