RFC: Execute upgrade scripts twice to ensure they are idempotent #57
Conversation
| @@ -11,6 +12,8 @@ function microserviceExists(microservice: "api" | "admin" | "site") { | |||
| return fs.existsSync(`${microservice}/package.json`); | |||
| } | |||
|
|
|||
| const isLocalDevelopment = !process.argv[1].includes("/_npx"); | |||
There was a problem hiding this comment.
This was the easiest way I found to check if the script is executed locally during development or via npx.
Maybe we can think of something smarter.
There was a problem hiding this comment.
Why doesn't process.env.NODE_ENV === "development" work? 🤔
| if (diffBefore !== diffAfter) { | ||
| console.error(`❌ ${script.name}: Changes detected when executing script twice`); | ||
| process.exit(-1); | ||
| } |
There was a problem hiding this comment.
Do we want / need this git diff check?
The problem is that some scripts rely on EsLint for cleanup. These scripts fail now, making it more complicated to implement scripts. E.g., replace-gridcoldef-import.ts just adds an import declaration in the end (no matter if it's already present), since ESLint will clean it up
There was a problem hiding this comment.
It's a nice idea, but I would still remove it, since upgrade scripts should be easy to write (otherwise devs won't do it). Also, it should be noticeable that a script doesn't by running it twice.
There was a problem hiding this comment.
If we want the git diff feature, we could move the second run to the single script run feature and diff after executing eslint
thomasdax98
changed the title
Alternative to #52
Description
Execute upgrade scripts twice in development to ensure they are idempotent (= the outcome is identical no matter how often they are executed).
I also added a
git diffcheck that checks if anything changed between the first and second execution. If changes are found, the script fails.see comments for open questions