Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix the default value of temp-dir in --help #3044

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gargnitingoogle
Copy link
Collaborator

@gargnitingoogle gargnitingoogle commented Feb 28, 2025

Description

Fix the default value of temp-dir in --help

Show the default value of temp-dir, which is "/tmp" in code, rather than keeping it ambiguous.

Link to the issue in case of a bug fix.

b/397660108

Testing details

  1. Manual
    $ git switch master && go run . --help | grep "\/tmp"
          --temp-dir string                              Path to the temporary directory where writes are staged prior to upload to Cloud Storage. (default: system default, likely /tmp)
    $ git switch - && go run . --help | grep "\/tmp"
          --temp-dir string                              Path to the temporary directory where writes are staged prior to upload to Cloud Storage. (default: "/tmp")
  2. Unit tests - NA
  3. Integration tests - NA

Show the default value of temp-dir, which is
"/tmp" in code, rather than keeping it
ambiguous.
@gargnitingoogle gargnitingoogle requested a review from a team as a code owner February 28, 2025 09:56
@kislaykishore kislaykishore requested review from a team, abhishek10004 and Tulsishah and removed request for a team and abhishek10004 February 28, 2025 09:56
Copy link

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.45%. Comparing base (260e33c) to head (d0b16cf).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3044   +/-   ##
=======================================
  Coverage   75.45%   75.45%           
=======================================
  Files         126      126           
  Lines       17724    17724           
=======================================
  Hits        13374    13374           
  Misses       3792     3792           
  Partials      558      558           
Flag Coverage Δ
unittests 75.45% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kislaykishore
Copy link
Collaborator

Thanks for the link, @gargnitingoogle. We also use tempdir to create anonymous files (link). That tempdir is resolved as per this that uses the TMPDIR environment variable if available and otherwise falls back to /tmp.
So, the ideal fixes here would be to align on using the same tempdir everywhere, infer it programmatically and then show it in the usage doc.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants