-
Notifications
You must be signed in to change notification settings - Fork 463
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
Add tsfmt support for maven #552
Conversation
…closer to the other defaults.
…both not working as expected.
private Map<String, Object> config; | ||
|
||
@Parameter(defaultValue = "${project.build.directory}", required = true, readonly = true) | ||
private File buildDir; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This default seems to not be effective...
|
||
if (buildDir == null) { | ||
buildDir = new File("."); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... because if it was, this wouldn't be necessary. I think part of what might be happening is that new File(".") is not resolving to where we suspect it to be. We could modify FormatterStepConfig
to include this value. It already has a FileLocator
, but unfortunately that can only be used to get files. Should be able to add the base directory to this, I imagine....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I tried this below and it is still not working.
The quick fix is to remove |
I have to do @source-knights, because apparently I can only ask for a review from a committer. |
Hey, no worries @nedtwigg . But not sure what you mean with push and comment? You mean in my original PR? Also I think that the error shown during the tsconfig test is misleading. From all I tested it actually find the files (cause the error with non existing files is different). If you look closely there is another error mentioned in the logs, and that might be the real error, just leading to the other error message. I mentioned that in my PR comments. Its about "-color" flag when calling the tsfmt. |
Perfect! Thanks. |
Here you can see how I refactored your tests to use the same files as the other tests. Both
tsconfigFile
andtsconfigInline
are failing.tsconfigFile
fails with the same "No inputs were found...", andtsconfigInline
is failing because it just isn't formatting at all. The others are passing.This indicates to me that there's something deeper that's wrong, and I have a suspicion which I'll elaborate on below.