Skip to content

Conversation

andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Jun 25, 2024

This is a follow-up to #280. Basically, that PR cherry-picked some fixes from main (unified_analytics v6) to a new branch meant to eventually be published as unified_analytics v5.8.8+2. However, since there were some API changes in v6, the tests didn't work (I already remembered to verify the tests right after I merged 😛).

This PR patches these tests. It makes LogHandler::logFile writable via a @visibleForTestingOnly setter, which I think is okay for patching an old major version.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@andrewkolos
Copy link
Contributor Author

andrewkolos commented Jun 25, 2024

The current plan is to wait and see whether @christopherfujino will be back in action tomorrow, since I don't think we really need (?) to get this published by the end of the week.

Once this merges—per https://github.com/dart-lang/tools/pull/280#issuecomment-2189866767—the next step would be to have devoncrew (or someone else with publishing permissions) publish this to pub.

@devoncarew
Copy link
Member

devoncarew commented Jun 25, 2024

Can you delete the first branches: line from https://github.com/dart-lang/tools/blob/main/.github/workflows/unified_analytics.yml#L6? I believe that will allow the tests to run for this PR.

This is to try to make CI run for this branch.
@github-actions github-actions bot added the type-infra A repository infrastructure change or enhancement label Jun 25, 2024
@devoncarew
Copy link
Member

You want the .github/workflows/unified_analytics.yml file :)

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewkolos andrewkolos merged commit accba5e into unified_analytics-v5.8.8+2 Jun 27, 2024
@andrewkolos andrewkolos deleted the patch-tests-for-588-2 branch June 27, 2024 17:42
@andrewkolos
Copy link
Contributor Author

@devoncarew the unified_analytics-v5.8.8+2 branch should be ready for publishing now

@devoncarew
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:unified_analytics type-infra A repository infrastructure change or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants