Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[url_launcher] Fix minor memory leak in Linux url-launcher tests #4648

Merged

Conversation

robert-ancell
Copy link
Contributor

@robert-ancell robert-ancell commented Jan 6, 2022

Fix minor memory leak in Linux url-launcher tests

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter]. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Oops! Thanks for the fix, LG other than needing a CHAGELOG update (using ## NEXT) to make publishable happy.

@robert-ancell
Copy link
Contributor Author

@stuartmorgan I updated the description to avoid the need for a CHANGELOG entry - should I add one for a change like this?

@stuartmorgan-g
Copy link
Contributor

The policy we'd settled on for now is to generally require a CHANGELOG even for non-production changes because it's convenient for us to see what changed (especially since occasionally we make changes we think aren't production-affecting but were wrong, although obviously that wouldn't apply here). We may revisit if it's annoying after living with it for a while.

I'm fine with not having one here, but you'll need to push something to get publish green; rerunning unfortunately doesn't work (I guess Cirrus is caching the description it had or something?)

@flutter-dashboard flutter-dashboard bot changed the base branch from master to main January 6, 2022 21:29
@flutter-dashboard
Copy link

This pull request was opened against a branch other than master. Since Flutter pull requests should not normally be opened against branches other than master, I have changed the base to master. If this was intended, you may modify the base back to master. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than master, unless this is an intentional hotfix/cherrypick.

@robert-ancell robert-ancell force-pushed the linux-url-launcher-test-leak branch from 2e67266 to bf59489 Compare January 6, 2022 21:29
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM

@stuartmorgan-g stuartmorgan-g added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Jan 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p: url_launcher platform-linux waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants