Add AI generated PRs to release notes. #977
Conversation
|
curl_close is deprecated, close is handled automatically in php > 8.0 |
| } | ||
| preg_match_all('!#([0-9]+)!', $pr->body, $issue_matches); | ||
| $preg_pattern = '!#([0-9]+)!'; | ||
| if (isset($pr->user->type) && $pr->user->type === 'Bot') { |
There was a problem hiding this comment.
There is a link on the PR to the original issue (on AI and human PRs as well)
There was a problem hiding this comment.
So can we remove this block of code, and look for the link. As also devs will be encouraged to drop the #123 in the body text
There was a problem hiding this comment.
@amitaibu Getting the issue number now via graphQL call, as it is not available via Rest API. I did not remove the existing function, but left it as a fallback if the Development section is not filled out.
mariano-dagostino
left a comment
mariano-dagostino
left a comment
There was a problem hiding this comment.
I think there is a bug when pulling the name of the pr. Tried this in another project and this is what I see:
- Autocomplete: Subject, country, city facet doesn't display [1.5h] (#2602)
- Fix autocomplete when the same word appears multiple times (#2620)
- Remove city from autocomplete search results (#2613)
- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> (#2610)
In the last case, the PR had a different name but requesting the review from copilot made the title of the PR incorrect.
mariano-dagostino
left a comment
mariano-dagostino
left a comment
There was a problem hiding this comment.
@balagan73 Thanks looks good for me. I tried in another project with correct results this time.
I added a few comments to some places where I see we can simplify the code.
| ]); | ||
|
|
||
| $ch = curl_init('https://api.github.com/graphql'); | ||
| curl_setopt($ch, CURLOPT_USERAGENT, 'Drupal Starter Release Notes Generator'); |
There was a problem hiding this comment.
Do we need this?
There was a problem hiding this comment.
Same thing we have at line 252.
| foreach ($linked_issues as $linked_issue) { | ||
| // Only include issues from the same repository. | ||
| $issue_repo = $linked_issue['repository']['name'] ?? ''; | ||
| $issue_owner = $linked_issue['repository']['owner']['login'] ?? ''; | ||
| if ($issue_repo !== $repo || $issue_owner !== $owner) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
I wonder if this check is really necessary as the graphql already specifies the repo.
There was a problem hiding this comment.
Most probably not, but as an edge case linked issues can be in a different repo. I think it doesn't hurt to have this extra check.
| } | ||
|
|
||
| $preg_pattern = '!#([0-9]+)!'; | ||
| if (isset($pr->user->type) && $pr->user->type === 'Bot') { |
There was a problem hiding this comment.
I wonder if the Bot check here is really necessary. As this is a fallback method maybe we should check the preg_pattern first and if no result check the second preg_patter.
#976