Skip to content
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

[CP] Roll dart_style 156f5c8c3bfe4e3b8e7adc95f7cf24d9500f11c6 into the SDK #52346

Closed
munificent opened this issue May 10, 2023 · 5 comments
Closed

[CP] Roll dart_style 156f5c8c3bfe4e3b8e7adc95f7cf24d9500f11c6 into the SDK #52346

munificent opened this issue May 10, 2023 · 5 comments
Assignees
@vsmenon @whesse @itsjustkevin @mit-mit @athomas
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable

Comments

@munificent
Copy link
Member

Commit(s) to merge

73dea24

Target

stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/302451

Issue Description

This couple of few formatting issues around records and pattern matching, and fixes a rare performance regression in long parameter lists.

The CL description has the full details.

What is the fix

Roll in a new commit of dart_style with the fixes.

Why cherry-pick

The formatting of some records and switch expressions is pretty bad without these fixes. Since there is very little of that code in the wild today, getting a fix out soon will minimize churn from users to write new code, get it formatted poorly, and then have the formatting improved later.

Risk

low

Issue link(s)

See below:

Extra Info

Here are the dart_style issues this fixes:

dart-lang/dart_style#1212
dart-lang/dart_style#1213
dart-lang/dart_style#1215
@munificent munificent added the cherry-pick-review Issue that need cherry pick triage to approve label May 10, 2023
@vsmenon
Copy link
Member

vsmenon commented May 10, 2023

lgtm

@itsjustkevin itsjustkevin added merge-to-stable cherry-pick-approved Label for approved cherrypick request labels May 11, 2023
@munificent
Copy link
Member Author

Should I wait for an LGTM on the CL too before landing it?

@itsjustkevin
Copy link
Contributor

Just waiting on it to be reviewed by someone!

@athomas
Copy link
Member

athomas commented May 12, 2023

I reviewed it.

Doc says:

Send the changelist for review. Await the appropriate consensus and approval via the cherry-pick-approved for them or any OWNER to review the changelist.

Once the cherry-pick issue is approved and the changelist is reviewed, the cherry-pick author will submit it to the commit queue.

copybara-service bot pushed a commit that referenced this issue May 12, 2023
@munificent
[stable] Roll latest dart_style into the SDK.
06b4ead 
The fixes are:

1. Comments in switch expressions would force the next line to split:

```
// Before:
return switch (level) {
  0 => a,
  // Comment.
  1
      when flag =>
    b,
};

// After:
return switch (level) {
  0 => a,
  // Comment.
  1 when flag => b,
};
```

2. Method calls on split record literals split before the ".":

```
// Before:
final a = (
  element,
  element,
)
    .getter;

// After:
final a = (
  element,
  element,
).getter;
```

3. Change how parameter metadata is formatted:

```
// Before:
function(
    @required
        int n,
    @deprecated('Some long deprecation string.')
        String s) {
  ...
}

// After:
function(
    @required int n,
    @deprecated('Some long deprecation string.')
    String s) {
  ...
}
```

This change also significantly improves performance in rare cases where
a function has many parameters.

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/302500
Change-Id: I56cd6ceb81ad9c98ca7f5d34eea05ef5d29228fb
Fixes: #52346
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302451
Reviewed-by: Alexander Thomas <[email protected]>
Commit-Queue: Bob Nystrom <[email protected]>
@munificent
Copy link
Member Author

Landed: https://dart-review.googlesource.com/c/sdk/+/302451.

@itsjustkevin itsjustkevin reopened this May 12, 2023
@itsjustkevin itsjustkevin added the cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. label May 15, 2023
@lrhn lrhn added the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vsmenon vsmenon

@whesse whesse

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@munificent @vsmenon @lrhn @whesse @itsjustkevin @mit-mit @athomas