-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: --output flag #169
feat: --output flag #169
Conversation
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.
Thanks for the contribution! I like the general idea, and the ability to specify output location, just have 2 small questions
help: | ||
'The output file to write the index to. Use "-" to write to stdout', |
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.
Do you have a use case for outputting to stdout? given the potential size and usage of these files, I'm curious how this might be used?
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 use SCIP internally for some stuff, and we sometimes have to save to a file just to read it straight back in to parse it. This way we can skip the write to disk
bin/scip_dart.dart
Outdated
@@ -71,5 +78,11 @@ Future<void> main(List<String> args) async { | |||
|
|||
final index = await indexPackage(packageRoot, packageConfig, pubspec); | |||
|
|||
File('index.scip').writeAsBytesSync(index.writeToBuffer()); | |||
print(result['output']); |
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'm assuming this was left in for debugging purposes, could we remove it?
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.
Oh, yep! Sorry 😅
QA +1
|
🚀 @Workiva/release-management-p 🚢 |
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.
+1 from RM
FYI @isaacharrisholt Thanks again for the contribution! |
Closes #168