Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
/pluginsPublic archive

[path_provider_linux] Using TMPDIR env as a primary temporary path#4218

Merged
stuartmorgan-g merged 8 commits intoflutter:masterfrom
proninyaroslav:patch-1
Sep 14, 2021
Merged

[path_provider_linux] Using TMPDIR env as a primary temporary path#4218
stuartmorgan-g merged 8 commits intoflutter:masterfrom
proninyaroslav:patch-1

Conversation

@proninyaroslav
Copy link
Contributor

@proninyaroslavproninyaroslav commentedAug 5, 2021•
edited by stuartmorgan-g
Loading

TMPDIR is a standard variable on UNIX/Linux systems, and is often used in containers such as Flatpak to redirect to a temporary folder inside a sandbox. This allows not to make hard bindings to the /tmp directory

Fixesflutter/flutter#87742

Pre-launch Checklist

  • I read theContributor Guide and followed the process outlined there for submitting PRs.
  • I read theTree Hygiene wiki page, which explains my responsibilities.
  • I read and followed therelevant style guides and ranthe auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does usedart format.)
  • I signed theCLA.
  • 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.
  • Iupdated pubspec.yaml with an appropriate new version according to thepub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel onDiscord.

TMPDIR is a standard variable on UNIX/Linux systems, and is often used in containers such as Flatpak to redirect to a temporary folder inside a sandbox. This allows not to make hard bindings to the /tmp directory
@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commentedAug 5, 2021•
edited
Loading

Thanks for the submission! This will need the remaining items in the checklist (other than doc comments) addressed before it moves forward with review. If you have any questions about completing those steps that aren’t addressed in the linked documentation, please let me know.

@proninyaroslav
Copy link
ContributorAuthor

@stuartmorgan
I have questions about this:

I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.I updated CHANGELOG.md to add a description of the change.

Do I have to make a version bump in this case?

@proninyaroslav
Copy link
ContributorAuthor

@stuartmorgan
About tests: the problem is that thePlatform class from thedart:io library is not mockable, so I can't insert the required environment variable value inside the test (Platform.environment). The solution would be to use the mockable version ofPlatform, some plugins use thishttps://pub.dev/packages/platform

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commentedAug 9, 2021•
edited
Loading

I have questions about this:

I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.I updated CHANGELOG.md to add a description of the change.

Do I have to make a version bump in this case?

The section you are quoting links to an explanation of the versioning policy for flutter/plugins. Could you elaborate on what your are seeing there that suggests you would not need to so that I can clarify it?

About tests: the problem is that thePlatform class from thedart:io library is not mockable, so I can't insert the required environment variable value inside the test (Platform.environment). The solution would be to use the mockable version ofPlatform, some plugins use thishttps://pub.dev/packages/platform

Since all you need is theenvironment, a much simpler solution at this point would just be to makePathProviderLinux take an injectable-by-tests environment map that would replace the call toPlatform.environment. We generally try to minimize package dependencies in the plugins here.

@proninyaroslav
Copy link
ContributorAuthor

@stuartmorgan
I added tests and also bumped the plugin version to 2.1.0 as I believe the API changes are backwards compatible and are not just a patch.

@proninyaroslav
Copy link
ContributorAuthor

@stuartmorgan
All comments have been resolved.

@proninyaroslav
Copy link
ContributorAuthor

@stuartmorgan
Can you review my changes again?

Copy link
Contributor

@stuartmorgan-gstuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. Looking again, the tests would be much clearer as three distinct tests rather than one test that alternates between changing the environment and then making new assertions. Rather than do another round of review, I just made those change in the PR.

LGTM with that. Thanks!

proninyaroslav reacted with thumbs up emoji
@stuartmorgan-gstuartmorgan-g added waiting for tree to go green(Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. and removed needs tests labelsSep 3, 2021
@stuartmorgan-g
Copy link
Contributor

Oops, this got caught by the Windows bot change so it was never auto-landed. Landing manually, since this can't affect Windows.

ditman reacted with thumbs up emoji

@stuartmorgan-gstuartmorgan-g removed 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. labelSep 14, 2021
@stuartmorgan-gstuartmorgan-g merged commit2076d1d intoflutter:masterSep 14, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull requestSep 14, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull requestSep 14, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull requestSep 14, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull requestSep 27, 2021
…lutter#4218)TMPDIR is a standard variable on UNIX/Linux systems, and is often used in containers such as Flatpak to redirect to a temporary folder inside a sandbox. This allows not to make hard bindings to the /tmp directoryFixesflutter/flutter#87742
KyleFin pushed a commit to KyleFin/plugins that referenced this pull requestDec 21, 2021
…lutter#4218)TMPDIR is a standard variable on UNIX/Linux systems, and is often used in containers such as Flatpak to redirect to a temporary folder inside a sandbox. This allows not to make hard bindings to the /tmp directoryFixesflutter/flutter#87742
@pugaizaipugaizai mentioned this pull requestFeb 1, 2024
8 tasks
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

1 more reviewer

@stuartmorgan-gstuartmorgan-gstuartmorgan-g approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[path_provider] Using TMPDIR env as a primary temporary path

3 participants

@proninyaroslav@stuartmorgan-g@stuartmorgan

Comments

[8]ページ先頭
©2009-2026 Movatter.jp