Skip to content

chore: enable lints in firebase_performance #5253

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

Merged
merged 3 commits into from
Mar 12, 2021
Merged

Conversation

rrousselGit
Copy link
Contributor

Description

Add extra lints for firebase_performance

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@google-cla google-cla bot added the cla: yes label Mar 7, 2021
@rrousselGit rrousselGit force-pushed the performance-lint branch 2 times, most recently from 5689192 to 8314764 Compare March 7, 2021 15:48
# the new lints that are already failing on this plugin, for this plugin. It
# should be deleted and the failing lints addressed as soon as possible.

include: ../../analysis_options.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

silly question probably, but why (if the all_lint_rules.yaml and analysis_options.yaml are effectively boilerplate duplicated amongst packages) don't they just refer to a common file in firebase_core or similar?

Copy link
Contributor Author

@rrousselGit rrousselGit Mar 7, 2021

Choose a reason for hiding this comment

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

The goal is, after these PRs are merged, I'll make a separate PR fusing all the duplicates in a single file at the root of the repo

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense as a pragmatic approach, I was sincerely just curious. Thanks @rrousselGit

@rrousselGit rrousselGit reopened this Mar 8, 2021
@rrousselGit
Copy link
Contributor Author

Merging since apparently the CI fail is not caused by this PR

cc @mikehardy

@rrousselGit rrousselGit merged commit ed00a84 into master Mar 12, 2021
@rrousselGit rrousselGit deleted the performance-lint branch March 12, 2021 11:39
@firebase firebase locked and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants