Skip to content
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

Don't set options.folder when --types-only #2253

Merged

Conversation

mikaelkaron
Copy link
Contributor

Closes #2251

@leorossi
Copy link
Contributor

Can you add a test?
I agree that line seems useless, also because after few lines that options is updated and that makes me think that should be tested

Closes platformatic#2251

Signed-off-by: Mikael Karon <mikael@karon.se>
@mikaelkaron mikaelkaron force-pushed the mikaelkaron-client-types-only-no-folder branch from e12fa03 to 506a8ab Compare March 12, 2024 17:15
Signed-off-by: Mikael Karon <mikael@karon.se>
@leorossi
Copy link
Contributor

I tried your branch, but the test passes also if I rollback your edit, can you please have a look? Thanks!

@mikaelkaron
Copy link
Contributor Author

mikaelkaron commented Mar 13, 2024

Just ran the test and I get this (if I revert my change):

[14:20:20.059] ERROR (70740): Command failed with exit code 2: /platformatic/node_modules/.pnpm/typescript@5.3.3/node_modules/typescript/bin/tsc --project /platformatic/packages/client-cli/test/tmp/platformatic-client-70740-1710336018054-5/tsconfig.json --rootDir .
plugin.ts(2,23): error TS2688: Cannot find type definition file for './uncanny/movies'.

@leorossi Can you take another look?

@mcollina
Copy link
Member

CI is failing.

@mikaelkaron
Copy link
Contributor Author

mikaelkaron commented Mar 13, 2024

CI is failing.

That is because the file is now generated in name/name.d.ts file like other cases. I can update the failing test to read the file from there, but we also have to update documentation here.

Signed-off-by: Mikael Karon <mikael@karon.se>
Signed-off-by: Mikael Karon <mikael@karon.se>
@mikaelkaron
Copy link
Contributor Author

So tests are still failing because of some timeout issue, I don't think it's related to this change?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit ffeee52 into platformatic:main Mar 14, 2024
88 of 90 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client ignores --folder when --types-only is used.
3 participants