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.

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

analyzer regression on pub.dartlang.org #35732

Closed
Sh1d0w opened this issue Jan 23, 2019 · 7 comments
Closed

analyzer regression on pub.dartlang.org #35732

Sh1d0w opened this issue Jan 23, 2019 · 7 comments

Comments

@Sh1d0w
Copy link

Sh1d0w commented Jan 23, 2019

Hi I am the maintainer of https://pub.dartlang.org/packages/multi_image_picker

Until today my package did not had any Health errors. However since today, I've released a new version, and the analyzer on pub.dartlang.org shows those problems:

screenshot 2019-01-23 at 12 30 13

However locally analyzer does not report any issues as it should be because I haven't included non_constant_identifier_names in my config

https://github.com/Sh1d0w/multi_image_picker/blob/master/analysis_options.yaml

For some reason it seems like pub.darlang.org ignores the package analysis_options and uses its own rules, which is regression from what we had before.

@zoechi
Copy link
Contributor

zoechi commented Jan 23, 2019

@isoos

@isoos
Copy link

isoos commented Jan 23, 2019

For some reason it seems like pub.darlang.org ignores the package analysis_options and uses its own rules, which is regression from what we had before.

package:pana and the pub site does use rule replacement, and we are doing it from the very beginning. This is to enforce a minimum/common set of rules, and not to rely on the ones that the package author provided. We also update the rules from time-to-time, following the toolset defaults.

In this case, your package is a Flutter plugin, and we use the defaults recommended for new Flutter projects to analyse it: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/analysis_options_user.yaml

Last week we've released a new version of pub site that used the updated rules, that's why you see the change. As the recommended defaults change over time, pana and pub site will follow it, and it could result in changes in the analysis results.

Looking at you code, it seems that you are using the final fields with uppercase, which is usually for classes. As the package is popular, I wouldn't do a single breaking update to it, rather a gradual one:

  • You can introduce a non-breaking migration path: renaming the fields to lowerCamelCase, and keeping a getter for the CamelCase versions.

  • You can use ignore rules to ignore a specific rule, clearing it for now, but hopefully addressing it later.

@Sh1d0w
Copy link
Author

Sh1d0w commented Jan 23, 2019

@isoos I see, thanks for the info and the suggestions. I will amend as per your recommendations.

Cheers!

@Sh1d0w Sh1d0w closed this as completed Jan 23, 2019
@zoechi
Copy link
Contributor

zoechi commented Jan 23, 2019

Does pub publish also check according to these rules?

@isoos
Copy link

isoos commented Jan 23, 2019

Does pub publish also check according to these rules?

Not yet, but hopefully we'll get there:
dart-lang/linter#1365

@Sh1d0w
Copy link
Author

Sh1d0w commented Jan 23, 2019

Does pub publish also check according to these rules?

I guess thats why I've got confused. Moreover the official reference on this topic does not reflect those recent changes, instead it shows the old rules available https://pub.dartlang.org/help#health

@pq
Copy link
Member

pq commented Jan 24, 2019

Sorry for the confusion @Sh1d0w. This is exactly what we're trying to head-off in the long-run.

I've added a few notes to dart-lang/linter#1365 with documentation ideas but this is really helpful context. Cheers!

/fyi @kwalrath

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants