================
@@ -45,13 +83,31 @@ def main(commit_sha: str, build_log_files: list[str]):
)
if advisor_response.status_code == 200:
print(advisor_response.json())
+ comments = [
+ get_comment(
+ github_token,
+ pr_number,
+ generate_test_report_lib.generate_report(
+ generate_test_report_lib.compute_platform_title(),
+ return_code,
+ junit_objects,
+ ninja_logs,
+ failure_explanations_list=advisor_response.json(),
----------------
boomanaiden154 wrote:
> I think these comments could get quite large, but then again we are only
> leaving at most 1. You might have to add some more size limits for the
> extreme cases but not worth doing until we see it happen in practice.
Yeah. We already have the size limit that will get applied here, but those can
still be large. I agree it's probably not worth doing until we see it in
practice. I don't think I've seen any PR with more than ~10 failures so far.
> I wonder if the premerge advisor content should also go to the build summary,
> but perhaps A: it already does or B: the advisor runs at a point where we've
> already submitted the build summary.
> Then again, having the build summary be very much "this is exactly what
> happened" and the comments on the PR be more "human" and maybe speculative
> makes some sense to me.
It doesn't currently go to the build summary. I think I'd like to keep the
build summary as exactly what happened for now. We can revisit that decision
based on user feedback. And yeah, a big part of the reason to surface the
advisor findings in a comment is because a lot of people do not realize the
summary view exists.
https://github.com/llvm/llvm-project/pull/166605
_______________________________________________
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
