refactor(core): Make instance role clearer (no-changelog) #10188
Conversation
n8n-assistant
bot
added
core
| @@ -917,12 +917,13 @@ export const schema = { | |||
| }, | |||
| }, | |||
|
|
|||
| instanceRole: { | |||
There was a problem hiding this comment.
why are we keeping this state in the config though? can this not be a property on one of the services instead?
we should stop using config to store data that user can't configure.
There was a problem hiding this comment.
I wouldn't feel comfortable making that change without thorough retesting.
There was a problem hiding this comment.
This was likely done following what was done with instanceType. I think there's value in making instanceRole clearer until we decide to remove all state from the schema. Let me know what you prefer.
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks! In principle it does, what I'm nervous about is merging this without thorough testing.
There was a problem hiding this comment.
The diff looks safe to me. Maybe we can ask someone else to review as well?
or, we can merge this PR, and then I can open a second PR?
There was a problem hiding this comment.
Let's bring it in. I'll do some spot checks testing tomorrow but I agree it looks safe.
Passing run #6181 ↗︎
Details:
Review all test suite changes for PR #10188 ↗︎ |
|||||||||||||||
|
✅ All Cypress E2E specs passed |
|
Got released with |
https://n8nio.slack.com/archives/C069HS026UF/p1721899050522709?thread_ts=1721894117.050629&cid=C069HS026UF