-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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(core): Move queueModeId
as hostId
to InstanceSettings
#11262
refactor(core): Move queueModeId
as hostId
to InstanceSettings
#11262
Conversation
n8n Run #7365
Run Properties:
|
Project |
n8n
|
Run status |
Passed #7365
|
Run duration | 04m 25s |
Commit |
d86bf917f6: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
|
Committer | Iván Ovejero |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
3
|
Pending |
0
|
Skipped |
0
|
Passing |
438
|
✅ All Cypress E2E specs passed |
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.
couple of nitpicks. LGTM otherwise
packages/cli/src/services/orchestration/main/multi-main-setup.ee.ts
Outdated
Show resolved
Hide resolved
* @example 'worker-nDJR0FnSd2Vf6DB5' | ||
* @example 'webhook-jxQ7AO8IzxEtfW1F' | ||
*/ | ||
readonly hostId: string; |
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.
should we setup member-ordering
linting rule to have consistent placement of properties in classes across the code?
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 always found this rule unnecessarily restrictive. I find it more important to be able to group fields and methods semantically rather than alphabetically.
✅ All Cypress E2E specs passed |
Got released with |
https://linear.app/n8n/issue/PAY-2027