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

chore: Move very_good_analysis to dev_dependencies #634

Merged
merged 2 commits into from
May 18, 2022
Merged

chore: Move very_good_analysis to dev_dependencies #634

merged 2 commits into from
May 18, 2022

Conversation

JCQuintas
Copy link
Contributor

@JCQuintas JCQuintas commented May 17, 2022

  • Dependency was on wrong place and prevented users of this package from upgrading to very_good_analysis: 3.0.0

@@ -14,7 +14,6 @@ dependencies:
provider: ^6.0.1
video_player: ^2.2.7
wakelock: ^0.6.1+1
very_good_analysis: ^2.4.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be moved to dev_dependencies instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not used. Having it or not doesn't make any difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can notice that because there is no entry on the pubspec.lock

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still though, if we decide to increase the analysis options in the future, we want to be able to pull lint rules from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then you are still able to add it when it is decided you want to use it?

If you really want it to be there even though it is not used I can add it, but it sounds a bit of a waste. I wouldn't recommend on installing libraries just in case it might be used in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a long list of housekeeping items that I will get to in the near future. Analysis is one of them. Just need to carve out the time for that very soon.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, there's no harm in leaving something in dev_dependencies that will never get exposed to end users, even if not really used yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No harm, but it can cause confusion. I've moved it to dev_dependencies

Choose a reason for hiding this comment

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

I agree with @JCQuintas. Adding a dependency which is not used makes no sense and confuse other developers.

Choose a reason for hiding this comment

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

If you want to use very_good_analysis later than add it when you need it. I see no advantage in adding a dependency which is not used.

@JCQuintas JCQuintas changed the title chore: remove unused dependency chore: Move very_good_analysis to dev_dependencies May 17, 2022
@diegotori
Copy link
Collaborator

LGTM.

@diegotori diegotori merged commit 1d57444 into fluttercommunity:master May 18, 2022
diegotori added a commit that referenced this pull request May 19, 2022
codenameakshay pushed a commit to qoohoo-app/chewie that referenced this pull request Aug 29, 2022
lg8294 added a commit to lg8294/chewie that referenced this pull request Sep 22, 2022
* commit '3948d23733638d8a58ea5ff065abfe96934d2373': (110 commits)
  Version 1.3.4. Addresses PRs fluttercommunity#623 and fluttercommunity#646.
  Update pubspec.yaml
  version updates
  fix: change video type
  fix: format and migration artifacts
  fix: apply diegotori suggestion
  feat: added support for apple and desktop version
  feat: Make progressIndicatorDelay a Duration
  fix: simplify assignation
  fix: name
  fix: let the android player buffer before seeking once more
  fix: removed old code
  fix: lint
  feat: added method to set progress delay
  feat: added example debug
  feat: add a delay before displaying progress indicator
  Version 1.3.3. Adds PR fluttercommunity#634.
  Update pubspec.yaml
  chore: remove unused dependency
  Version 1.3.2. Adds PR fluttercommunity#626.
  ...

# Conflicts:
#	example/ios/Runner.xcodeproj/project.pbxproj
#	example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
#	example/ios/Runner/Info.plist
#	example/lib/main.dart
#	example/pubspec.yaml
#	lib/src/chewie_player.dart
#	pubspec.yaml
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

Successfully merging this pull request may close these issues.

3 participants