-
Notifications
You must be signed in to change notification settings - Fork 19.7k
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(editor): Chat Trigger tweaks #9618
feat(editor): Chat Trigger tweaks #9618
Conversation
Signed-off-by: Oleg Ivaniv <[email protected]>
Signed-off-by: Oleg Ivaniv <[email protected]>
Signed-off-by: Oleg Ivaniv <[email protected]>
Signed-off-by: Oleg Ivaniv <[email protected]>
Signed-off-by: Oleg Ivaniv <[email protected]>
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.
LGTM
@@ -118,7 +118,7 @@ export abstract class DirectoryLoader { | |||
|
|||
if (currentVersionNode.hasOwnProperty('executeSingle')) { | |||
throw new ApplicationError( | |||
'"executeSingle" has been removed. Please update the code of this node to use "execute" instead!', | |||
'"executeSingle" has been removed. Please update the code of this node to use "execute" instead.', |
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.
We have a rule n8n-nodes-base/node-param-description-excess-final-period
for not adding a final period in single-sentence node descriptions. Maybe we should apply this rule to those errors as well.
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.
It's a good point, I've asked David if we should keep the final period or not. Thanks!
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.
As per David's request, I've removed the final period for single-sentence errors. Could you re-approve, please?
2 flaky tests on run #5305 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
NDV > should not retrieve remote options when required params throw errors |
Screenshots
Video
|
|
NDV > Stop listening for trigger event from NDV |
Screenshots
Video
|
Review all test suite changes for PR #9618 ↗︎
✅ All Cypress E2E specs passed |
Signed-off-by: Oleg Ivaniv <[email protected]>
cd02994
to
8eccf59
Compare
1 flaky test on run #5307 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
NDV > should not retrieve remote options when required params throw errors |
Screenshots
Video
|
Review all test suite changes for PR #9618 ↗︎
✅ All Cypress E2E specs passed |
1 flaky test on run #5312 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
NDV > should not retrieve remote options when required params throw errors |
Screenshots
Video
|
Review all test suite changes for PR #9618 ↗︎
Got released with |
Summary
Related tickets and issues
Review / Merge checklist
(no-changelog)
otherwise. (conventions)