YousefED / typescript-json-schema

Generate json-schema from your Typescript sources
BSD 3-Clause "New" or "Revised" License
3.17k stars 323 forks source link

use const instead of enum #535

Closed lublak closed 1 year ago

lublak commented 1 year ago

Please:

fixes https://github.com/YousefED/typescript-json-schema/issues/534

domoritz commented 1 year ago

Please remove the npm lock file

lublak commented 1 year ago

@domoritz done

domoritz commented 1 year ago

Tests are failing

lublak commented 1 year ago

@domoritz can you try again

domoritz commented 1 year ago

Why should we make this configurable? I feel it's a better default and would like it always enabled. What do you think?

lublak commented 1 year ago

@domoritz then it is not compatible with your tests. (backwards compatible?)

domoritz commented 1 year ago

Of we think this is better then we can release a new breaking version.

One question? How does this work if you have an enum with one value?

domoritz commented 1 year ago

And speaking of tests, we need one.

lublak commented 1 year ago

@domoritz the issue ist currenlty: The complete project doesn't work. If i just try npm test, the tests are currently failing (and no i have not really time to fix all issues). Okay a breaking version can be fine for this. If you have a enum with one value it works fine with most stuff. But const is here a better solution. After all, that's what const is for.

domoritz commented 1 year ago

You can just add one test and run that wither by marking the test as .only or commenting out the others.

lublak commented 1 year ago

@domoritz i changed now the tests and its now a breaking change.

domoritz commented 1 year ago

Please fix the tests so we can merge this.

lublak commented 1 year ago

@domoritz yes! The issue is, if i run my test local i can not see all issues directly. So i only can see if its fails here. My local test looks like this (in the workflow the tests works fine, currently also fails but with correct error message):

grafik grafik

but i hope i can fix it without testing it.

lublak commented 1 year ago

@domoritz can you try again? :)