[tool] Restore publishability#11745
Conversation
…ng change standpoint
There was a problem hiding this comment.
Code Review
This pull request refactors the repository tooling to support global installation and improves repository root detection by replacing hardcoded paths with a dynamic discovery mechanism. It introduces a centralized .repo_tool_cache for managing external Java and Kotlin formatters and updates the documentation to include setup instructions for both local and global usage. Review feedback highlights the need for robust error handling during formatter downloads, improved repository root detection to accommodate git worktrees, and the correction of syntax errors in the README's shell examples. Additionally, a concern was raised regarding side effects within the cache directory resolution utility.
| final http.Response response = await http.get(_javaFormatterUrl); | ||
| javaFormatterFile.writeAsBytesSync(response.bodyBytes); | ||
| file.writeAsBytesSync(response.bodyBytes); |
There was a problem hiding this comment.
The HTTP request lacks error handling. If the download fails (e.g., due to a 404 error or network issues), the tool will write the error response body to the JAR file, causing obscure errors when it later attempts to execute it. Verify the status code before writing.
final http.Response response = await http.get(_javaFormatterUrl);
if (response.statusCode != 200) {
print('Failed to download Google Java Format (error ${response.statusCode})');
throw ToolExit(1);
}
file.writeAsBytesSync(response.bodyBytes);| final http.Response response = await http.get(_kotlinFormatterUrl); | ||
| kotlinFormatterFile.writeAsBytesSync(response.bodyBytes); | ||
| file.writeAsBytesSync(response.bodyBytes); |
There was a problem hiding this comment.
The HTTP request lacks error handling. Verify the status code before writing the bytes to the file.
final http.Response response = await http.get(_kotlinFormatterUrl);
if (response.statusCode != 200) {
print('Failed to download ktfmt (error ${response.statusCode})');
throw ToolExit(1);
}
file.writeAsBytesSync(response.bodyBytes);
tarrinneal
left a comment
There was a problem hiding this comment.
Not sure if I like the alias swap, but I guess it's more readable.
Restores the repo tooling to a publishable state, so that we can start using it for flutter/core-packages.
I expect fixes will be necessary to make it actually work correctly in that repo, but this gives us a foundation to iterate from where we can actually publish and test changes:
publish_to: noneFixes flutter/flutter#185764