-
Notifications
You must be signed in to change notification settings - Fork 188
test: Add failing tests for [object Object] identifier bug in migration operations #1471
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
Conversation
They are all falling into the same "module" |
|
Mostly yeah, besides those:
node-pg-migrate/src/operations/functions/createFunction.ts Lines 47 to 49 in f4b4ab4
|
|
Mh, okay. There are already 8 touched files. We should keep the diff small. 10-20 file changes would already count as a medium to big PR. |
|
I think I'm separating this PR due to different fixes, So we can either:
IMO, the first option is far better since the worst thing that could happen is in case someone did it So, operators are easy, but the CREATE TABLE "myschema"."distributors" (
"user_id" integer CONSTRAINT "[object Object]_fk_user_id" REFERENCES "myschema"."users"
);
COMMENT ON CONSTRAINT "[object Object]_fk_user_id" ON "myschema"."distributors" IS $pga$user reference$pga$;This actually works because
(
throw new Error(
`Language for function ${mOptions.literal(functionName)} have to be specified`
);node-pg-migrate/eslint.config.ts Line 147 in e0a2c37
This rule could catch it, I think we can enable it back once we fix them, I only noticed because I'm running sonar locally so it is asking me to confirm on every push 😂 (actually this is happening with
I've uploaded the analysis from my fork to SonarCloud |
|
@brenoepics did you saw the PR #1458 from @adrenalin? |
Added failing test cases for each issue reported at #1469, will be fixing them soon, let me know if I should send them in the same PR or separately.