-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: add temperature and output file options to search command #21
Conversation
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.
Hey! contributor, thank you for opening a Pull Request 🎉.
Soon one of our maintainers will review it and provide you with feedback/suggestions. If you think it's something urgent, feel free to reach out Pradumna Saraf on Twitter. Star ⭐ this repo to show us support.
Happy, Open Source!
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.
Hey,
Thanks for the PR. I don’t think you’ve gone through the contributing guidelines. We don’t accept any PRs unless an issue has been raised, triaged, and assigned to you.
That said, I won’t enforce that rule this time 😉 since I’ve seen you supporting my work on social media—thank you for that!
Great ideas! I’ve added a review.
…ame specification
two flags has been added |
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 think it can further be simplified. What I mean is if the user enters a file name with the --save
command, we will write to that file otherwise, create an output.txt
by default in the current dir. I don't think there is a need to do the --output
flag on top of it. We can just rename the flag to --save-output
for better understanding.
…prove file handling
and it will not work for
|
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.
My bad by mistake, I hit the close PR button.
Oh yes, you are correct. Then we can do something like this:
searchCmd.Flags().BoolVarP(&saveOutput, "save", "s", false, "Save the output to a file")
searchCmd.Flags().StringVarP(&outputFile, "output", "o", "output.txt", "Output file name")
So for gencli ... --save
, it will save to output.txt
as it's the default value for the --output
flag,
otherwise if gencli ... --save --output "filename.txt"
we will save to the user given input.
…nd specify output file name
i have add the features that you told. |
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.
Hey, I made a couple of changes.
- Added the temperature method. I think by mistake, you removed it.
model.SetTemperature(temperature)
- Also, I learned we need not to set the variable default value. At the time of execution, the program will pick up the default value from the flag itself.
+ numWords string
+ outputLanguage string
+ temperature float32
- numWords string = "150"
- outputLanguage string = "english"
- temperature float32 = 0.7
- Change model temp to
0.5
You can have a look and let me know if you need to make any further changes.
…ature default value
Sorry, temperature was removed by mistake, and it's great no further changes are required. |
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 to merge. Thank you for your contribution!
Issue
Closes #22
👨💻 Changes proposed
--temperature
(-t
): Controls response creativity (0.0-1.0)--save
(-s
): Saves response to specified file✔️ Check List
CliVersion
variable incmd/version.go
file.📄 Note to reviewers