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

In the Event.exception constructor, modify a clone of the args argument instead of args itself #1201

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Nov 21, 2024

Hotfix for #1195 for package:unified_analytics v6. This is somewhat necessary because the current flutter beta depends on v6.

Note that merging this PR as-is won't result in a publish. This PR exists for the sake of code review.


  • 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.

This is a follow-up to #280 (comment). In summary, we only run CI checks on PRs against the main branch. However, in cases where an older major version of a package needs to be patched, it doesn't make sense to PR against main. CI won't run for these PRs, but it probably should.

This updates our GitHub CI configs to run checks against PRs against any branch.

---

<details>
  <summary>Contribution guidelines:</summary><br>

- See our [contributor guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md) for general expectations for PRs.
- Larger or significant changes should be discussed in an issue before creating a PR.
- Contributions to our repos should follow the [Dart style guide](https://dart.dev/guides/language/effective-dart) and use `dart format`.
- Most changes should add an entry to the changelog and may need to [rev the pubspec package version](https://github.com/dart-lang/sdk/blob/main/docs/External-Package-Maintenance.md#making-a-change).
- Changes to packages require [corresponding tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing).

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
</details>
@github-actions github-actions bot added the type-infra A repository infrastructure change or enhancement label Nov 21, 2024
@andrewkolos
Copy link
Contributor Author

andrewkolos commented Nov 21, 2024

@devoncarew

Do you know if updating

# A CI configuration to auto-publish pub packages.
name: Publish
on:
pull_request:
branches: [ main ]
push:
tags: [ '[A-z]+-v[0-9]+.[0-9]+.[0-9]+' ]

to run against any branch would work for publishing patches to older versions of packages? Assuming that would work, do we want it to (are there branch protections that we have on main that we would then want on all branches)? I'm happy to track this in a separate issue if we don't want to solve it today.

@andrewkolos andrewkolos marked this pull request as ready for review November 21, 2024 18:34
@andrewkolos
Copy link
Contributor Author

andrewkolos commented Nov 21, 2024

Note for @kenzieschmoll: you can ignore the .github/workflows config file changes. This was cherry-picked from main just to get this branch tested on CI.

@devoncarew
Copy link
Member

Hi!

Do you know if updating to run against any branch would work for publishing patches to older versions of packages? Assuming that would work, do we want it to (are there branch protections that we have on main that we would then want on all branches)? I'm happy to track this in a separate issue if we don't want to solve it today.

I think we want to:

  • create a branch from the tag of unified_analytics that we want to patch (https://github.com/dart-lang/tools/tags)
  • apply the changes, push it up in a PR for review for merge into that branch
  • not try to use the publishing automation to publish that new patch, but publish locally from disk

Our publishing automation isn't really set up to handle publishing older patches; that's a rare enough occurrence that I think it's reasonable to handle it by manually publishing from a branch from disk.

@andrewkolos
Copy link
Contributor Author

SGTM. The first step is already satisfied here, so I will publish this myself once this gets an approval.

@andrewkolos andrewkolos merged commit 4c2ff99 into unified_analytics-v6.1.6 Nov 21, 2024
13 checks passed
@andrewkolos andrewkolos deleted the patch-exception-on-616 branch November 21, 2024 20:25
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