Skip to content

Add supported Luci builders for cocoon repo #875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
keyonghan merged 6 commits into from
Jul 31, 2020
Merged

Add supported Luci builders for cocoon repo #875

keyonghan merged 6 commits into from
Jul 31, 2020

Conversation

@keyonghan
Copy link
Contributor

@keyonghan keyonghan commented Jul 28, 2020

Related issue: flutter/flutter#62429

@keyonghan keyonghan requested a review from godofredoc July 28, 2020 23:02
godofredoc
godofredoc reviewed Jul 29, 2020
View reviewed changes
Copy link
Contributor

@godofredoc godofredoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a readme file with information about the structure of the folder, the expected format of the json file and a test that validates the json validity and content?

@keyonghan
Copy link
Contributor Author

keyonghan commented Jul 29, 2020

Can we add a readme file with information about the structure of the folder, the expected format of the json file and a test that validates the json validity and content?

README added.
Didn't find an easy way to test json validity from pre-submit tests, but added corresponding logic to check json format in utils, and added tests in utiles_test.dart to cover that.

@keyonghan keyonghan mentioned this pull request Jul 29, 2020
Merged
godofredoc
godofredoc reviewed Jul 31, 2020
View reviewed changes
Copy link
Contributor

@godofredoc godofredoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change? do we need to land the files in the different repositories first?

dev/README.md Outdated
Copy link
Contributor

@godofredoc godofredoc Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To trigger LUCI presubmit tasks?

Copy link
Contributor Author

@keyonghan keyonghan Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

dev/cocoon_try_builders.json Outdated
Copy link
Contributor

@godofredoc godofredoc Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the test I was thinking something like a dart script that receives a the path of the json file as a flag and validates it has the correct content and the file is parseable.

Copy link
Contributor Author

@keyonghan keyonghan Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a separate dart file to validate the json contents: validate_json.dart, and included it in the README.

Copy link
Contributor

@godofredoc godofredoc Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a separate dart file to validate the json contents: validate_json.dart, and included it in the README.

Great, maybe not in this PR but can we run the validation in presubmit?

Copy link
Contributor Author

@keyonghan keyonghan Jul 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try adding that in a separate PR.

@keyonghan
Copy link
Contributor Author

keyonghan commented Jul 31, 2020

Is this a breaking change? do we need to land the files in the different repositories first?

No, this is not.
Existing cocoon is still reading build coinfigs from /app_dart/dev/luci_try_builders.json.

@keyonghan keyonghan merged commit e8c6c81 into flutter:master Jul 31, 2020
@keyonghan keyonghan deleted the add_luci_builder branch March 13, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@godofredoc godofredoc godofredoc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@keyonghan @godofredoc