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

Closed
lxl66566 opened this issue Aug 2, 2024 · 8 comments · Fixed by #1018
Closed

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

lxl66566 opened this issue Aug 2, 2024 · 8 comments · Fixed by #1018
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

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Sep 30, 2024
@cswpy
Copy link

cswpy commented Sep 30, 2024

Have been occupied by other stuff, will work on this later.

@lxl66566 lxl66566 removed the Stale label Sep 30, 2024
@Standing-Man
Copy link

Hi @lxl66566, I'm interested in contributing to this issue. Let me give it a try and work on solving it.

@lxl66566
Copy link
Collaborator Author

lxl66566 commented Oct 9, 2024

@JianMinTang Thank you for your enthusiasm!
@cswpy Thank you for your previous dedication! Are you still planning to work on this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment