-
Notifications
You must be signed in to change notification settings - Fork 49
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 option --out-dir to packge vector_graphics_compiler #215
Conversation
if (Platform.isWindows) { | ||
outputPath = '${outDir.path}\\${file.path.split("\\").last}.vec'; | ||
} else { | ||
outputPath = '${outDir.path}/${file.path.split("/").last}.vec'; |
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.
Use package:path's join method instead.
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.
Please add a test and use path.join
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.
LGTM, thanks!
@jonahwilliams fyi
(assuming tests pass haha) |
@@ -15,6 +15,7 @@ dependencies: | |||
path_parsing: ^1.0.1 | |||
xml: ^6.3.0 | |||
vector_graphics_codec: 1.1.8 | |||
path: ^1.8.3 |
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.
Doesn't this add a dependency on path to all users of flutter_svg?
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.
ahhh I think we cheated on this before and put it in dev_dependencies...
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.
We could also use a more relaxed constraint?
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.
I think you can cheat if its only used in the bin directory, might want to double check. I'd be fine with that.
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.
If that doesn't work, I'd be fine with a loose constraint like we have
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.
I changed this to ^1.8.0. I'll fix up the analysis issues unrelated to this patch separately.
add new option --out-dir to specific output directory when specific --input-dir option