-
Notifications
You must be signed in to change notification settings - Fork 370
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
refactor(chat): Remove parenthesis wrapping of other files when calling selectedCodePromptWithExtraFiles
#7022
base: main
Are you sure you want to change the base?
Conversation
@vovakulikov I'm going to need some help with the tests that fail because the polly recordings will need to be updated. |
This aligns the output of these commands with the output we get from the default prompts.
7de6d67
to
7401a14
Compare
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'm unsure about this. What we probably need is a better prompt which asks to explain/document/etc. the and then mentions the rest of the file as context later in the request.
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.
More feedback inline.
I think you should not drop those extra context items from smell and explain.
…uested in review - Now they also show up in the prompt, no longer being added as context without showing up as duplicate context chip - Simplified `selectedCodePromptWithExtraFiles` return as instructed by @dominiccooney
@dominiccooney I've addressed your suggestions in a48d316 I really hope that as @kalanchan is prioritizing the changelog generator, a similar effort can be put into giving someone like myself a script or a bot command that I can run to update without needing extra permissions or software in order to avoid having to ask another dev todo this for me. |
- Per instructions in ticket feedback
refactor(chat): Remove file mention duplication for default commands.
Based on the comments:
#6910 (review)
#6910 (comment)
I'm opening this pull request in order to get rid of the extra setup parentheses between the primary mention and the other mentions.
For extra context on reason see this comment #6910 (comment)
I am also removing the duplication of context mention from the default commands because these commands have been duplicated as default prompts and they do not duplicate the context mention leaving just the context mention with the file range.
Test plan