Backend support branching for flutter repo by keyonghan · Pull Request #658 · flutter/cocoon

keyonghan · 2020-02-25T00:05:50Z

This PR supports Flutter to be a branch based build, test and release process.

Right now, master, beta, and stable are supported.

tvolkert · 2020-02-25T01:04:07Z

app_dart/lib/src/request_handlers/refresh_github_commits.dart

We're going to be using a release branch scheme, where each dev (x.y.z) release gets its own branch, and testing is done on those branches. Then dev, beta, and stable will just point to hashes that have already been tested as part of the release branch.

So rather than enumerating the branches here, you'll want to query GitHub for the list of branches, then filter that list to include only branches that match a specified list of patterns (e.g. ['master', r'v[0-9]+.[0-9]+.[0-9]+']).

I see there are existing branches: vx.x.x-hotfixes in flutter repo. Shall we exclude those, namely only strictly matching vx.x.x pattern?

You can exclude them. We'll want a way to update the pattern of the branches we match though, since the proposal for those branch names is still in flux. Maybe a config file that we query so we can update the pattern without needing to update Cocoon?

Should we add a top level folder called release to both flutter/flutter and flutter/engine with a file called branch_regs.txt with one regex per line?

I'd put it somewhere in the dev/ folder hierarchy.

tvolkert · 2020-03-02T21:44:57Z

app_dart/lib/src/request_handlers/refresh_github_commits.dart

-    final List<Commit> newCommits = <Commit>[];
-    final List<Map<String, Object>> tableDataInsertAllRequestRows =
-        <Map<String, Object>>[];
+    final RegExp exp = RegExp(r'v[0-9]+.[0-9]+.[0-9]+');


No need to explicitly exclude the existing hotfix branches - just change the regexp to be stricter - r'^v[0-9]+\.[0-9]+\.[0-9]+$'

Thanks for the suggestion.

tvolkert · 2020-03-02T21:52:21Z

app_dart/lib/src/service/github_service.dart

+    final PaginationHelper paginationHelper = PaginationHelper(github);
+
+    final List<dynamic> commits = <dynamic>[];
+    await for (Response response in paginationHelper.fetchStreamed(


May be worth sending a PR to package:github to add the functionality to query commits on a branch.

Will consider sending that soon.

tvolkert

Approach LGTM.

This should probably also contain a change to the endpoint that drives the dashboard commit status table, so as to only list commits on a specific branch (defaulting to master).

keyonghan · 2020-03-03T00:38:51Z

Approach LGTM.

This should probably also contain a change to the endpoint that drives the dashboard commit status table, so as to only list commits on a specific branch (defaulting to master).

@tvolkert added issue to track: flutter/flutter#51807

godofredoc · 2020-03-03T00:51:59Z

app_dart/dev/branch_regexps.txt

@@ -0,0 +1,2 @@
+^v[0-9]+.[0-9]+.[0-9]+$


This file has to go into the different supported repos, e.g flutter/flutter and flutter/engine

That was my first reaction as well, but I couldn't think of a reason why it needs to... @godofredoc is there a reason why?

One possibility is to support different branches per repo(not sure this is really necessary) the other one will be give more control to the people working on the different repos on which branches will be tested.

The main reason I added this comment is to be able to load it without a new deployment but it will be ok to keep it here and loading it over http.

Yeah living here and requesting over http sgtm

tvolkert · 2020-03-03T01:12:47Z

app_dart/lib/src/model/appengine/commit.dart

  String repository;

+  /// The branch of the commit.
+  @StringProperty(propertyName: 'Branch', required: true)


How does making this required play with the existing entries that don't have it?

For the safety purpose, removing required. Thanks.

tvolkert · 2020-03-03T01:13:50Z

app_dart/lib/src/request_handlers/refresh_github_commits.dart

-    final List<Commit> newCommits = <Commit>[];
-    final List<Map<String, Object>> tableDataInsertAllRequestRows =
-        <Map<String, Object>>[];
+    final File file = File('dev/branch_regexps.txt');


This will require a new Cocoon deployment to pick up the new file. I think we'll have to request it from GitHub over http.

Considering moving the file to flutter or engine repo, will change to http request.

tvolkert · 2020-03-03T01:14:41Z

app_dart/dev/branch_regexps.txt

@@ -0,0 +1,2 @@
+^v[0-9]+.[0-9]+.[0-9]+$


dots should be preceded by backslashes, or the regexp is saying "any character"

keyonghan · 2020-03-03T01:35:23Z

nit: against.

@godofredoc Fixed. Thanks.

tvolkert · 2020-03-03T03:53:03Z

app_dart/lib/src/request_handlers/refresh_github_commits.dart

+        }
+
+        await Future<void>.delayed(gitHubBackoffCalculator(attempt));
+        return <String>['master'];


This belongs outside the for loop

Moved outside. Thank you for pointing this out.

tvolkert · 2020-03-03T03:53:36Z

app_dart/lib/src/request_handlers/refresh_github_commits.dart

+      client.close(force: true);
+    }
+
+    log.error('GitHub not responding; giving up');


This is unreachable

tvolkert

LGTM

keyonghan requested review from dnfield, godofredoc and tvolkert February 25, 2020 00:06

tvolkert reviewed Feb 25, 2020

View reviewed changes

support branching

dcf936d

keyonghan force-pushed the branch_support branch from fa2490a to dcf936d Compare February 28, 2020 00:37

query github for branch list

e4d88ad

tvolkert reviewed Mar 2, 2020

View reviewed changes

keyonghan added 2 commits March 2, 2020 15:49

create branch regexp file

a3c5e12

fix test error

897ac9a

keyonghan changed the title ~~Support branching for flutter repo~~ Backend support branching for flutter repo Mar 3, 2020

keyonghan mentioned this pull request Mar 3, 2020

Support branching for flutter repo flutter/flutter#51807

Closed

19 tasks

add readme

a08b3b7

godofredoc reviewed Mar 3, 2020

View reviewed changes

tvolkert reviewed Mar 3, 2020

View reviewed changes

keyonghan added 2 commits March 2, 2020 19:25

read branch_regexps.txt file over http

877ce7d

leave branch_regexps.txt in cocoon

7131881

tvolkert reviewed Mar 3, 2020

View reviewed changes

tvolkert approved these changes Mar 3, 2020

View reviewed changes

keyonghan added 2 commits March 2, 2020 21:08

for loop error fix

e6ae0ee

fix return

451a6f4

keyonghan merged commit 94f3e65 into flutter:master Mar 3, 2020

keyonghan mentioned this pull request Mar 3, 2020

Fix timezone and dir for branch_regexps.txt - branching support #664

Merged

keyonghan deleted the branch_support branch March 13, 2024 20:16

Conversation

keyonghan commented Feb 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvolkert Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvolkert left a comment

Choose a reason for hiding this comment

Uh oh!

keyonghan commented Mar 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keyonghan commented Mar 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvolkert left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tvolkert Mar 2, 2020 •

edited

Loading