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

Update dependencies and remove unused ones. Pin git dependencies. #169

Merged
merged 2 commits into from
Sep 16, 2022

Conversation

ewanas
Copy link
Contributor

@ewanas ewanas commented Sep 16, 2022

OP#2200


Code Review Checklist

  • Description accurately reflects what changes are being made.
  • Description explains why the changes are being made (or references an issue containing one).
  • The PR appropriately sized.
  • New code has enough tests.
  • New code has enough documentation to answer "how do I use it?" and "what does it do?".
  • Existing documentation is up-to-date, if impacted.

@donpui
Copy link
Contributor

donpui commented Sep 16, 2022

Got warning on Android:

Warning: The plugin permission_handler_android requires Android SDK version 33.
For more information about build configuration, see https://docs.flutter.dev/deployment/android#reviewing-the-build-configuration.
One or more plugins require a higher Android SDK version.
Fix this issue by adding the following to /Users/donataspuidokas/Development/destiny/android/app/build.gradle:
android {
  compileSdkVersion 33
  ...
}

However it compiled and installed.

Copy link
Contributor

@donpui donpui left a comment

Choose a reason for hiding this comment

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

I have done smoke test on Windows, Android, Linux, Mac. Seems it works.

Copy link
Collaborator

@meejah meejah left a comment

Choose a reason for hiding this comment

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

Whenever possible (i.e. in non-3rd-party repos) it'd be way better to actually tag things (then used named tags here).

file_picker: ^4.5.0
intro_slider:
git:
url: https://github.com/shareef-dweikat/flutter-intro-slider
ref: a9bb492412c68419900750fe12e0487e3a1168fb
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we do releases of these? or at least named tags (since they appear to be repos "we" control?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's reasonable, I agree, CC @shareef-dweikat

I put the hashes here as an improvement over nothing at all. I guess also tags may be overwritten to point to different hashes. In that sense, maybe hashes are more reproducible?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I guess depends what the use-case is ... for reproducibility I suppose the hash is better here (coming from python-land, you'd pin against "the hash of the 1.2.3 release on PyPI" normally).

Even if this uses the hash it could still be beneficial to have like "22.9.1" (date-based) tags to help the humans but I'll leave that to people actually doing this ;) and also I don't know flutter ecosystem at all

@ewanas
Copy link
Contributor Author

ewanas commented Sep 16, 2022

Merging to allow testing early while intro_slider and expand_widget get updated(Looks like they are in need of getting some upstream updates as well as dependency updates) and also have a release or at least a tag, then will update those in another PR.

For window_size we will keep the hash as it is an external dependency.

@ewanas ewanas merged commit 57cbbda into main Sep 16, 2022
@ewanas ewanas deleted the dependencies branch September 16, 2022 18:13
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