Skip to content

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Aug 22, 2022

Redo #109475 to work around stale luci-flutter updates #110023.

Limited cherry-pick of #109428 that only has the deprecation method replacement without the other lint fixes. Limited the cp to just this line to reduce risk.

Re lack of testing:

We have good test coverage of both these methods, but they are running on a stable version of Ruby in our CI (I think it's just using the macOS default system version 2.6).
Getting a preview version of Ruby into our CI system doesn't seem feasible.

See test-exemption from original PR #109428 (comment)

test-exempt: for practical reasons. testing this is likely to cost more than it saves since it's an entirely new ecosystem. #109431 in case we change out minds.

Fixes #109476

@jmagman jmagman added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 22, 2022
@jmagman jmagman self-assigned this Aug 22, 2022
@flutter-dashboard
Copy link

This pull request was opened against a branch other than master. Since Flutter pull requests should not normally be opened against branches other than master, I have changed the base to master. If this was intended, you may modify the base back to flutter-3.2-candidate.5. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than master, unless this is an intentional hotfix/cherrypick.

@flutter-dashboard flutter-dashboard bot changed the base branch from flutter-3.2-candidate.5 to master August 22, 2022 21:44
@flutter-dashboard flutter-dashboard bot added engine flutter/engine related. See also e: labels. framework flutter/packages/flutter repository. See also f: labels. labels Aug 22, 2022
@jmagman jmagman changed the base branch from master to flutter-3.2-candidate.5 August 22, 2022 21:44
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@jmagman jmagman removed framework flutter/packages/flutter repository. See also f: labels. engine flutter/engine related. See also e: labels. labels Aug 22, 2022
@XilaiZhang XilaiZhang added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 22, 2022
Copy link
Contributor

@itsjustkevin itsjustkevin left a comment

Choose a reason for hiding this comment

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

LGTM

@XilaiZhang XilaiZhang merged commit 0961626 into flutter:flutter-3.2-candidate.5 Aug 22, 2022
@jmagman jmagman deleted the cp-exists-ruby branch August 23, 2022 00:38
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CP] Replace removed deprecated Ruby method in tool helper script

3 participants