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

[feature]: replace hardcoding "/tmp/xxx" path #933

Open
lxl66566 opened this issue Aug 2, 2024 · 4 comments · May be fixed by #974
Open

[feature]: replace hardcoding "/tmp/xxx" path #933

lxl66566 opened this issue Aug 2, 2024 · 4 comments · May be fixed by #974
Assignees
Labels
good first issue Good for newcomers

Comments

@lxl66566
Copy link
Collaborator

lxl66566 commented Aug 2, 2024

currently we use many /tmp/xxx temp directory on db testing, and needs manually delete the dir in the end of test.
It's better to use some crate like tempfile to help us.

pros:

  • no need to delete folders manually. programmers may forget to add remove code after testing.
  • no need to worry about folder name collision. each test will always use different folder.
  • although xline only supports test on linux, it's not bad to add some cross-platform ability.

things to do:

  • add crate to [dev-dependencies] and replace /tmp/xxx path
  • for those using /tmp/xxx path not in test mod, ex. crates/xline/src/server/command.rs, use std::env::temp_dir instead.
    • for this ex. case, we need to define a function to generate snapshot path from uuid. It's not a good idea to use hardcoded string in coding.
@lxl66566 lxl66566 added the good first issue Good for newcomers label Aug 2, 2024
Copy link

github-actions bot commented Aug 2, 2024

👋 Thanks for opening this issue!

Reply with the following command on its own line to get help or engage:

  • /contributing-agreement : to print Contributing Agreements.
  • /assignme : to assign this issue to you.

@cswpy
Copy link

cswpy commented Aug 29, 2024

Hello there, I am new to the project and would love to contribute! I’d love to take a stab at this one.

@liangyuanpeng
Copy link
Contributor

liangyuanpeng commented Aug 29, 2024

feel free to comment /assignme for assign it, and welcome :)

@cswpy
Copy link

cswpy commented Aug 30, 2024

/assignme

@cswpy cswpy linked a pull request Aug 31, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants