Add dart define from file #2232
Conversation
github-actions
bot
added
the
package: patrol_cli
| Map<String, dynamic> fromConfigFile({required String path}) { | ||
| final filePath = _fs.path.join(_projectRoot.path, path); | ||
| final file = _fs.file(filePath); | ||
|
|
||
| if (!file.existsSync()) { | ||
| return {}; | ||
| } | ||
|
|
||
| final jsonString = file.readAsStringSync(); | ||
| final json = jsonDecode(jsonString) as Map<String, dynamic>; | ||
| return json; | ||
| } | ||
|
|
There was a problem hiding this comment.
If the decision was made to completely parse the files into separate define's, then I would like it to work the same way as in flutter_tools:
- fail if file is missing
- do not add
project Root.pathat beginning of file path - support more than just
JSON(maybe someone is using this)
How this is implemented in flutter_tools:
https://github.com/flutter/flutter/blob/64dd1ca9bc32fbc767a3f63e4c9c5ae2fc355380/packages/flutter_tools/lib/src/runner/flutter_command.dart#L1452-L1494
There was a problem hiding this comment.
Thanks. Good notice! I reused flutter_tools code for parsing. You can check it out now
There was a problem hiding this comment.
Won't this test cause all workflows with patrol tests in our repository to fail? I think we should exclude this test from all of them
There was a problem hiding this comment.
You're right. Need to manage that
Done @mateuszwojtczak |
|
|
||
| if (dartDefines.isNotEmpty) { | ||
| dartDefines.forEach((key, value) { | ||
| modified[key] = value as String; |
There was a problem hiding this comment.
@pdenert
I tried this change in our project and realized that not everything is in the form of strings, so the code crashes here - I suspect that other projects may have a similar problem
There was a problem hiding this comment.
You're right. Can you create issue for that? We will try to address this ASAP
Close #1742