[file_selector] Annotate all creation of XTypeGroup with // ignore: prefer_const_contructor #6463
Conversation
There was a problem hiding this comment.
Thanks for tackling this!
I see there are a couple of PRs for this, and that's just covering two packages so far; it's probably easiest for something that's rote like this (and thus easy to review) to make one large PR covering the entire plugin, rather than doing it per-package. Since it doesn't require publishing, there's no dependency issue the way there would be for API changes.
eugerossetto
force-pushed
the
111906-ignore-const-constructor-file-selector
branch
from
f79e340 to
f760ea6
Compare
eugerossetto
requested review from
cbracken,
jmagman and
ditman
as code owners
| @@ -15,6 +15,8 @@ class OpenImagePage extends StatelessWidget { | |||
|
|
|||
| Future<void> _openImageFile(BuildContext context) async { | |||
| // #docregion SingleOpen | |||
| // TODO(stuartmorgan): https://github.com/flutter/flutter/issues/111906 | |||
There was a problem hiding this comment.
For the entries in #docregion sections, let's use ignore_for_file: at the top of the file instead to get them out of the README excerpts. That's generally not how we want to do ignores, but this will be very short-lived.
There was a problem hiding this comment.
You've marked this as resolved, but did the opposite of what I requested. As I said, we don't want these TODOs in the README.
| @@ -27,4 +27,3 @@ dev_dependencies: | |||
| sdk: flutter | |||
| mockito: ^5.1.0 | |||
| pigeon: ^3.2.5 | |||
|
|
|||
There was a problem hiding this comment.
Please revert this file so we don't have to do a version check override.
There was a problem hiding this comment.
You've marked this as resolved, but didn't change anything.
stuartmorgan-g
added
the
override: no changelog needed
|
Why not disable the lint, here, and then reenable + fix the affected files? To me, that sounds like half of the effort of "ignoring everywhere" then "un-ignoring + fixing" everywhere. There's a risk that non-const things might land in the meantime, but they'll eventually get fixed when the lint gets re-enabled. /cc @stuartmorgan? |
|
That's a really big hammer; I'd rather we avoid things that could introduce problems in unrelated packages. (The third option would have been to temporarily change the custom analysis options for the If we have to do this again for another package, we can revisit the approach, but I thought added boilerplate in exchange for added safety during the process was worthwhile. Now that the PR is basically done, I don't think it's worth changing since reverting the PR later should be trivial (since locally it can be an actual revert, followed by fixes.) |
|
Hi guys, we fixed the readme_excerpts check. We believe that, with this change, all the comments have been resolved. |
| @@ -27,4 +27,3 @@ dev_dependencies: | |||
| sdk: flutter | |||
| mockito: ^5.1.0 | |||
| pigeon: ^3.2.5 | |||
|
|
|||
There was a problem hiding this comment.
You've marked this as resolved, but didn't change anything.
| @@ -15,6 +15,8 @@ class OpenImagePage extends StatelessWidget { | |||
|
|
|||
| Future<void> _openImageFile(BuildContext context) async { | |||
| // #docregion SingleOpen | |||
| // TODO(stuartmorgan): https://github.com/flutter/flutter/issues/111906 | |||
There was a problem hiding this comment.
You've marked this as resolved, but did the opposite of what I requested. As I said, we don't want these TODOs in the README.
|
Hi Stuart, sorry for the misunderstanding, we applied the suggested changes. |
stuartmorgan-g
added
the
autosubmit
… ignore: prefer_const_contructor (flutter/plugins#6463)
eugerossetto
deleted the
111906-ignore-const-constructor-file-selector
branch
…refer_const_contructor (flutter#6463)
…refer_const_contructor (flutter#6463)
Annotate all creation of XTypeGroup with // ignore: prefer_const_constructor to prepare this implementation for the const constructor of XTypeGroup.
#111906 [file_selector]
XTypeGroupshould be immutablePre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.