Skip to content

Conversation

@FMorschel
Copy link
Contributor

@FMorschel FMorschel commented Oct 16, 2024

This is simply removing unnecessary parenthesis from various places inside the packages. This change is because of a change to the unnecessary_parentesis lint that will trigger in these places. Here is the CL https://dart-review.googlesource.com/c/sdk/+/390161.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

If anything else is needed please let me know.

I'd like to ask for this PR to wait a bit until the bots are run again on that CL so that I can be sure nothing else will trigger, I will come back here and update this whenever everything is complete. Thanks!

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

The change in its current form LGTM, but you'll need to update the changelog.

@FMorschel
Copy link
Contributor Author

The change in its current form LGTM, but you'll need to update the changelog.

Do I add the change to the closest CHANGELOG.md? There are a lot of examples changed here and no example folder has a changelog.

Or do I only need to add it to the cases where the change is outside the example folder?

@cbracken
Copy link
Member

I've been working on Flutter nearly a decade and I'll be honest, until yesterday I didn't know either (evidence).

The example changes won't need anything, but CI is likely complaining about the following:

So I suspect bumping the patch version here:

and adding a changelog entry here:

should be sufficient.

@FMorschel FMorschel marked this pull request as ready for review October 16, 2024 18:19
@cbracken
Copy link
Member

Looks like this test needs fixing:

00:08 �[32m+294�[0m�[31m -1�[0m: test/version_test.dart: pigeon version matches pubspec �[1m�[31m[E]�[0m�[0m                                                                                                                              
  Expected: '22.5.1'
    Actual: '22.5.0'
     Which: is different.
            Expected: 22.5.1
              Actual: 22.5.0
                           ^
             Differ at offset 5
  
  package:matcher              expect
  test/version_test.dart 17:5  main.<fn>

Looking at this line in the pubspec.yaml, you'll want to update this line in generator_tools.dart.

@FMorschel
Copy link
Contributor Author

Thanks. I had completely ignored that comment.

@stuartmorgan-g stuartmorgan-g added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Oct 17, 2024
@stuartmorgan-g
Copy link
Collaborator

CHANGELOG/version override: While this technically changes some production code, the change is a no-op other than to silence a warning. (Technically the example/lib/main.dart changes should have a version change under our policy since they are published to pub.dev, but the change is trivial enough that we can just pick that up on the next publish event.)

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2024
@auto-submit auto-submit bot merged commit e973bf3 into flutter:main Oct 17, 2024
76 checks passed
@FMorschel FMorschel deleted the unnecessary-parenthesis branch October 17, 2024 10:27
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 17, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 17, 2024
flutter/packages@a35f02d...5582669

2024-10-17 [email protected] [two_dimensional_scrollables] Fixes TreeViewNode collapsing not working (flutter/packages#7474)
2024-10-17 [email protected] Revert "[in_app_purchase_storekit] Add support for purchase and transactions #7574" (flutter/packages#7886)
2024-10-17 [email protected] [camera_android] Mark `description` in `sendCameraErrorEvent` as `@NonNull` (flutter/packages#7877)
2024-10-17 [email protected] [image_picker_web] Loosen mime dep to >=1.0.4 <3.0.0 (flutter/packages#7879)
2024-10-17 [email protected] Removing unnecessary parenthesis (flutter/packages#7881)
2024-10-17 [email protected] [in_app_purchase_storekit] Add support for purchase and transactions #7574 (flutter/packages#7812)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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 override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: camera p: google_maps_flutter p: pigeon p: webview_flutter platform-android platform-ios platform-macos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants