Skip to content

Commit e57a9ce

Browse files
authored
Refactoring. Lower number github api call. (#4879)
Refactoring while working on flutter/flutter#178426
1 parent b4471a3 commit e57a9ce

File tree

1 file changed

+19
-38
lines changed

1 file changed

+19
-38
lines changed

app_dart/lib/src/request_handlers/github/webhook_subscription.dart

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -571,12 +571,19 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
571571

572572
Future<void> _checkForTests(PullRequestEvent pullRequestEvent) async {
573573
final pr = pullRequestEvent.pullRequest!;
574+
// We do not need to add test labels if this is an auto roller author.
575+
if (config.rollerAccounts.contains(pr.user!.login)) {
576+
return;
577+
}
574578
final eventAction = pullRequestEvent.action;
575579
final slug = pr.base!.repo!.slug();
576580
final isTipOfTree = pr.base!.ref == Config.defaultBranch(slug);
577581
final gitHubClient = await config.createGitHubClient(pullRequest: pr);
578582
await _validateRefs(gitHubClient, pr);
579583
if (kNeedsTests.contains(slug) && isTipOfTree) {
584+
log.info(
585+
'Applying framework repo labels for: owner=${slug.owner} repo=${slug.name} and pr=${pr.number}',
586+
);
580587
switch (slug.name) {
581588
case 'flutter':
582589
final isFusion = slug == Config.flutterSlug;
@@ -587,15 +594,16 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
587594
gitHubClient,
588595
eventAction,
589596
pr,
590-
isFusion: isFusion,
591-
files: files,
597+
files,
598+
slug,
592599
);
593600
if (isFusion) {
594601
await _applyEngineRepoLabels(
595602
gitHubClient,
596603
eventAction,
597604
pr,
598-
files: files,
605+
files,
606+
slug,
599607
);
600608
}
601609
case 'packages':
@@ -607,23 +615,14 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
607615
Future<void> _applyFrameworkRepoLabels(
608616
GitHub gitHubClient,
609617
String? eventAction,
610-
PullRequest pr, {
611-
bool isFusion = false,
612-
List<PullRequestFile>? files,
613-
}) async {
618+
PullRequest pr,
619+
List<PullRequestFile> files,
620+
RepositorySlug slug,
621+
) async {
614622
if (pr.user!.login == 'engine-flutter-autoroll') {
615623
return;
616624
}
617625

618-
final slug = pr.base!.repo!.slug();
619-
log.info(
620-
'Applying framework repo labels for: owner=${slug.owner} repo=${slug.name} isFusion=$isFusion and pr=${pr.number}',
621-
);
622-
623-
files ??= await gitHubClient.pullRequests
624-
.listFiles(slug, pr.number!)
625-
.toList();
626-
627626
final labels = <String>{};
628627
var hasTests = false;
629628
var needsTests = false;
@@ -668,11 +667,6 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
668667
);
669668
}
670669

671-
// We do not need to add test labels if this is an auto roller author.
672-
if (config.rollerAccounts.contains(pr.user!.login)) {
673-
return;
674-
}
675-
676670
if (!hasTests &&
677671
needsTests &&
678672
!pr.draft! &&
@@ -754,15 +748,15 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
754748
Future<void> _applyEngineRepoLabels(
755749
GitHub gitHubClient,
756750
String? eventAction,
757-
PullRequest pr, {
758-
List<PullRequestFile>? files,
759-
}) async {
751+
PullRequest pr,
752+
List<PullRequestFile> files,
753+
RepositorySlug slug,
754+
) async {
760755
// Do not apply the test labels for the autoroller accounts.
761756
if (pr.user!.login == 'skia-flutter-autoroll') {
762757
return;
763758
}
764759

765-
final slug = pr.base!.repo!.slug();
766760
var hasTests = false;
767761
var needsTests = false;
768762

@@ -772,9 +766,6 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
772766

773767
var engineFiles = 0;
774768

775-
files ??= await gitHubClient.pullRequests
776-
.listFiles(slug, pr.number!)
777-
.toList();
778769
for (var file in files) {
779770
final path = file.filename!;
780771
if (isFusion && _isFusionEnginePath(path)) {
@@ -800,11 +791,6 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
800791
return;
801792
}
802793

803-
// We do not need to add test labels if this is an auto roller author.
804-
if (config.rollerAccounts.contains(pr.user!.login)) {
805-
return;
806-
}
807-
808794
if (!hasTests &&
809795
needsTests &&
810796
!pr.draft! &&
@@ -882,11 +868,6 @@ final class GithubWebhookSubscription extends SubscriptionHandler {
882868
}
883869
}
884870

885-
// We do not need to add test labels if this is an auto roller author.
886-
if (config.rollerAccounts.contains(pr.user!.login)) {
887-
return;
888-
}
889-
890871
if (!hasTests &&
891872
needsTests &&
892873
!pr.draft! &&

0 commit comments

Comments
 (0)