-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[pigeon] reorg generator files #8532
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
Conversation
And I do believe this is exempt from versioning |
This is actually a breaking change. The Dart package convention is that anything in If we don't want the generator files to be directly imported (which I think we don't), we should do a breaking change to move a bunch of things into |
I bumped the version and moved everything into src. I can wait to do this along with another breaking change if you'd like, but I think editing the files and moving them is a bit messy for reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with changelog nit.
I can wait to do this along with another breaking change if you'd like, but I think editing the files and moving them is a bit messy for reviews.
I expect ~0 people to actually be broken by this change, and we've always treated major version changes for pigeon
as low-cost, so I don't think there's any real value in combining (and as you point out, not-trivial downsides for our ability to do code reviews).
@@ -4,7 +4,7 @@ | |||
|
|||
import 'dart:io' show exit; | |||
|
|||
import 'package:pigeon/pigeon_cl.dart'; | |||
import 'package:pigeon/src/pigeon_cl.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, I guess bin/
has special-case allowance in the analyzer to import from the same package's src
even if it's using a package:
import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was surprised it was allowed as well, since the pigeon_cl.dart file is used outside of lib and tests, should I move it back out of src?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, I found a reference later on the page I had linked to, it's explicitly allowed:
You are free to import libraries that live in lib/src from within other Dart code in the same package (like other libraries in lib, scripts in bin, and tests) but you should never import from another package's lib/src directory.
And that's the preferred format for such an include:
How you import libraries from within your own package depends on the locations of the libraries:
When reaching inside or outside lib/ (lint: avoid_relative_lib_imports), use package:.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't push the button again. I need a Chrome extension for this.
We should land #8302 first to avoid a ton of churn on that pr |
flutter/packages@02c6fef...e6ce02c 2025-02-05 65381000+raju8000@users.noreply.github.com [vector_graphics] Allow transition between placeholder and loaded image to have an animation (flutter/packages#8195) 2025-02-04 andrechalella@gmail.com [flutter_markdown] Make custom table column alignments work when text wraps (flutter/packages#8340) 2025-02-04 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Adds internal wrapper for iOS native `IMAAdPodInfo` (flutter/packages#8429) 2025-02-03 tarrinneal@gmail.com [pigeon] reorg generator files (flutter/packages#8532) 2025-02-03 byoungchan.lee@gmx.com [pigeon] [swift] Fix `PigeonError` sendability conformance in Swift 6 (flutter/packages#8302) 2025-02-03 engine-flutter-autoroll@skia.org Roll Flutter from b007899 to 8e2a6fc (61 revisions) (flutter/packages#8556) 2025-02-03 20989940+aednlaxer@users.noreply.github.com [google_maps_flutter] Support for Ground Overlay - platform interface (flutter/packages#8518) 2025-01-31 737941+loic-sharma@users.noreply.github.com [tool] Add --xcode-warnings-exceptions flag (flutter/packages#8524) 2025-01-31 stuartmorgan@google.com [tool] Ensure that injected dependency overrides are sorted (flutter/packages#8542) 2025-01-31 jonahwilliams@google.com [vector_graphics] Revert leak tracker change (flutter/packages#8544) 2025-01-31 parlough@gmail.com [shared_preferences_tool] Loosen vm_service constraint to allow for 15 (flutter/packages#8539) 2025-01-31 32538273+ValentinVignal@users.noreply.github.com [in_app_purchase] Activate leak testing for android (flutter/packages#8369) 2025-01-31 cunderw@gmail.com [flutter_markdown] Allow tables to be scrollable with IntrinsicColumnWidth (flutter/packages#8526) 2025-01-30 goderbauer@google.com Update CODEOWNERS for pkg:animations (flutter/packages#8534) 2025-01-30 engine-flutter-autoroll@skia.org Roll Flutter from c1ffaa9 to b007899 (43 revisions) (flutter/packages#8527) 2025-01-30 pawel.jakubowski@leancode.pl [video_player_web] Adjust Web implementation to the new platform interface (flutter/packages#8528) 2025-01-30 tarrinneal@gmail.com [shared_preferences] Exposed SharedPreferencesOptions. (flutter/packages#8530) 2025-01-29 stuartmorgan@google.com Re-land [shared_preferences] Add shared preferences devtool (flutter/packages#8531) 2025-01-29 84978733+alejandro-all-win-software@users.noreply.github.com [in_app_purchase_storekit] Add Swift Package Manager compatibility (flutter/packages#8469) 2025-01-29 stuartmorgan@google.com Revert "Re-land [shared_preferences] Add shared preferences devtool" (flutter/packages#8529) 2025-01-29 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Fixes trailing `?` by comparing iterables (flutter/packages#8521) 2025-01-29 737941+loic-sharma@users.noreply.github.com [tool] Refactor args of strings or YAML file lists (flutter/packages#8513) 2025-01-28 48155875+Michae1Weiss@users.noreply.github.com [go_router] Add missing await keyword to onTap callback in the code example in `navigation.md` (flutter/packages#8343) 2025-01-28 stuartmorgan@google.com Re-land [shared_preferences] Add shared preferences devtool (flutter/packages#8519) 2025-01-28 32538273+ValentinVignal@users.noreply.github.com [vector_graphics] Fix memory leaks and activate leak testing [prod-leak-fix] (flutter/packages#8373) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Just moves a few files into folders to clean things up a bit. Future organizing is likely, but I wanted to move the files with no changes first
Just moves a few files into folders to clean things up a bit. Future organizing is likely, but I wanted to move the files with no changes first
Just moves a few files into folders to clean things up a bit. Future organizing is likely, but I wanted to move the files with no changes first