nit: Add a short comment explain path dependencies - #2511
Conversation
amirh
left a comment
amirh
left a comment
There was a problem hiding this comment.
Thanks!
Left 2 nits.
You may also want to update the plugin template here: https://github.com/flutter/flutter/blob/5f4658973be1ad10a59a0a278b6cdf41b10fe7f6/packages/flutter_tools/templates/app/pubspec.yaml.tmpl#L43
| sdk: flutter | ||
| android_alarm_manager: | ||
| path: ../ | ||
| android_alarm_manager: {path: ../} # ^x.y.z in your application |
There was a problem hiding this comment.
I really don't care if this is one line or 2 lines, though I don't want these to go back-and-forth between the 2 styles.
This is the kind of things we should write in a style guide, the closest thing to a style guide I found are the examples here: https://dart.dev/tools/pub/dependencies (which are using 2 lines). If we believe this style is preferred we should probably update that page as well.
There was a problem hiding this comment.
I only used the one-liner style because it put the comment on the same line.
| sdk: flutter | ||
| android_alarm_manager: | ||
| path: ../ | ||
| android_alarm_manager: {path: ../} # ^x.y.z in your application |
There was a problem hiding this comment.
nit: I'd make the comment more verbose so it's easier to understand why it's a path dependency here, maybe something like:
The example app is bundled with the plugin so we use a path dependency on the parent directory to use the current plugin's version. When depending on this package from a real application you should use : plugin_name: ^x.y.z
See https://dart.dev/tools/pub/dependencies#version-constraints for more information.
There was a problem hiding this comment.
Good point, that's better than using one-line :)
d3f50fa to
03ee13f
Compare
amirh
left a comment
amirh
left a comment
There was a problem hiding this comment.
LGTM
Version checker is complaining as we did not bump package versions, I'm ok with landing this without publishing assuming people mostly check the examples in GitHub and not from the pub tarball, though it's an interesting thought to run by @jonasfj 😄
|
@amirh, I just updated the PR to include They aren't meant to be published I assume, and the template already has these: |
cfd78db to
be20c9f
Compare
|
@amirh, I've updated this and flutter/flutter#50978, if you're still happy with this let's ship it :) |
|
Please merge if you're happy with this. |
|
hi @jonasfj could you please rebase & take a look at the CI errors? |
be20c9f to
21c6066
Compare
|
Not sure why CI would fail. |
…ve#3093) * Convert initial Polyline/Polygon points correctly. * Convert Flutter Color to CSS correctly. Fixes flutter/flutter#65152 Fixes flutter/flutter#67032
…archive#3120) * Android Code Inspection and Clean up * Format
github-actions
Bot
added
p: google_sign_in
p: image_picker
p: in_app_purchase
p: integration_test
p: ios_platform_images
p: local_auth
p: package_info
p: path_provider
p: plugin_platform_interface
p: quick_actions
p: sensors
p: share
p: shared_preferences
p: url_launcher
p: video_player
p: webview_flutter
|
Okay, I'm going to close this... Not sure what mess I just made of the history 🙈 |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
@jonasfj If you want to do a clean version I can review it ASAP so it doesn't bitrot. |
20 participants
Adds a comment
# ^x.y.z in your applicationwhere examples have path dependencies.This was motivated by dart-lang/pub#1850 (comment) where a user is running into problems by copying the dependency from an
example/pubspec.yaml.Maybe we shouldn't make it all one-line... and maybe we could phrase the comment better.
Or maybe we shouldn't even do this -- feel free to close this if you think this is too obvious :)