-
Notifications
You must be signed in to change notification settings - Fork 35
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 CODEOWNERS for package:unified_analytics #289
Conversation
I consider this package to be un-owned. I (and @christopherfujino) have only contributed to it recently only to address critical If this change is mostly necessary just to make CI happy, I suppose I'm okay with my name being used here. cc @christopherfujino @jtmcdole for thoughts |
We do need this to be owned generally; its published in a dart publisher and core to several of our tools. When this was shipped initially Elias was the owner of record with the Flutter tools team providing backup ownership (ala b/267635605). |
And the context, the CODEOWNERS file is not specifically intended to indicate ownership here as much as just cc specific people for PRs affecting portions of the repo's codebase. |
It sounds like this is a cross-tool package for shared logic. I believe we need more than one volunteer from an understaffed team to show real commitment. I'm not sure how Elias reported into the team, but he was the only one that said flutter tools would back it up. What team can help share the burden? |
He was on the flutter tools team; and I do always get buy-in from a team's TL before publishing a package through the dart publishers - individual SWEs always move on.
Yes, I believe that's correct. Used by flutter tools, the dart cli, devtools, and the analysis server? There is a periodic analytics sync, so perhaps ownership could be discussed there. The SLO for the published package is "This is not intended to be general purpose or consumed by the community. It is responsible for toggling analytics collection for related tooling on each developer's machine", so I think I'm mostly concerned with who would be on the hook for P0s. |
I checked in with the VM team and there are some threads I can track down as well. Thanks for keeping us honest! |
Agreed.
I see—thanks for the link.
Well said. To be clear, I am happy to continue contributing this package (especially as it pertains to the flutter CLI tool). However, I feel being the owner could result in pains down the line that I would like to think about sooner rather than when it happens. Looking at the note of the last unified analytics sync meeting, it looks like ownership was discussed, but I don't see any conclusion written down. I assume there was none. I can attend the next sync and bring it up again then (in a few weeks).
I agree with this practice. Still, the reason I'm hesitant here is how dramatically the Flutter tools team staffing situation changed since this was figured out. Consider the original two authors. Elias moved on, and Chris stepped up to lead eng prod efforts. The tools team made a choice to be the owner of the package then, but it did not have much of a choice when it came to circumstances that have changed between then and now.
I can take on this responsibility for now, and hopefully this can be addressed in the next analytics sync. Sorry for the noise. |
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 caveats. See prior conversation.
Thanks! I'll land this as is - it'll be good to have a current name here. Please feel free to update as and when ownership discussions happen.
Yup, definitely agree that things have changed significantly since this was published. |
Revisions updated by `dart tools/rev_sdk_deps.dart`. ecosystem (https://github.com/dart-lang/ecosystem/compare/f977423..2719d0c): 2719d0c 2024-08-08 Devon Carew add invalid_runtime_check_with_js_interop_types, unintended_html_in_doc_comment (dart-lang/ecosystem#285) http (https://github.com/dart-lang/http/compare/73fce77..76512c4): 76512c4 2024-08-07 Kate test(http_client_conformance_tests): Remove old skips (dart-lang/http#1284) d7ae256 2024-08-08 Anikate De [docs] sort pkg list in ascending order (dart-lang/http#1287) b82d88c 2024-08-06 Anikate De [docs] Add ok_http entry to readme (dart-lang/http#1285) package_config (https://github.com/dart-lang/package_config/compare/f0b7256..76934c2): 76934c2 2024-08-06 Kevin Moore Latest lints, require Dart 3.4 (dart-lang/package_config#157) sync_http (https://github.com/dart-lang/sync_http/compare/ab8377e..91c0dd5): 91c0dd5 2024-08-12 dependabot[bot] Bump actions/checkout from 4.1.6 to 4.1.7 (google/sync_http.dart#49) test (https://github.com/dart-lang/test/compare/9fbbfdb..8be3c94): 8be3c948 2024-08-12 Ömer Sinan Ağacan Run dart2wasm integration test on Windows (dart-lang/test#2265) e656e5a9 2024-08-12 Yaroslav Vorobev fix: use `toFilePath` in package config Uri (dart-lang/test#2262) 6bfe0d62 2024-08-12 Ömer Sinan Ağacan Fix documentation rendering issues (dart-lang/test#2264) tools (https://github.com/dart-lang/tools/compare/55dbd6e..d563c38): d563c38 2024-08-13 Moritz Add health workflow (dart-lang/tools#292) 8ac5509 2024-08-12 Devon Carew Update CODEOWNERS for package:unified_analytics (dart-lang/tools#289) web (https://github.com/dart-lang/web/compare/e89fe49..4996dc2): 4996dc2 2024-08-12 Srujan Gaddam Ignore unintended_html_in_doc_comment (dart-lang/web#278) Change-Id: I808778af5fb9a1f6885ae847614ffb660fcb8662 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380204 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Auto-Submit: Devon Carew <devoncarew@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@andrewkolos, are you or @christopherfujino the best owner(s) for this package?
Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.