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

refactor file cache code #25

Merged
merged 1 commit into from
May 10, 2023

Conversation

rfyiamcool
Copy link
Contributor

@rfyiamcool rfyiamcool commented May 7, 2023

what

  1. add rwlock for file cache, to ensure thread-safe.
  2. refactor file_test.go by assert mode.
  3. mkdirall dir when new file cachego.

@rfyiamcool rfyiamcool changed the title adjust file cache code refactor file cache code May 7, 2023
Copy link
Owner

@faabiosr faabiosr left a comment

Choose a reason for hiding this comment

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

Thanks you so much for you contribution, however I added some points regarding the changes. Also because is such a small change, I recommend you to squash all commits, and follow the commit message instructions. https://github.com/faabiosr/cachego/blob/master/CONTRIBUTING.md#create-a-commit

@rfyiamcool rfyiamcool force-pushed the fix/file_thread_safe branch from e7163a6 to 04b2095 Compare May 7, 2023 12:11
@rfyiamcool
Copy link
Contributor Author

Thanks you so much for you contribution, however I added some points regarding the changes. Also because is such a small change, I recommend you to squash all commits, and follow the commit message instructions. https://github.com/faabiosr/cachego/blob/master/CONTRIBUTING.md#create-a-commit

oh, done, i merge multiple commit to one commit.

@rfyiamcool rfyiamcool force-pushed the fix/file_thread_safe branch from 04b2095 to 5e4c52a Compare May 7, 2023 12:18
Copy link
Owner

@faabiosr faabiosr left a comment

Choose a reason for hiding this comment

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

Regarding the last commented about the removed tests. Also I recommend you to rebase this PR with the latest master changes (made by you 😄 ).

@rfyiamcool rfyiamcool force-pushed the fix/file_thread_safe branch from 5e4c52a to 0283906 Compare May 10, 2023 00:24
@faabiosr faabiosr self-requested a review May 10, 2023 09:28
@faabiosr
Copy link
Owner

@rfyiamcool Did you rebase your branch with the latest changes?

@faabiosr faabiosr merged commit 4d7e104 into faabiosr:master May 10, 2023
@faabiosr
Copy link
Owner

By the way, thanks for the great contribution @rfyiamcool !

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

Successfully merging this pull request may close these issues.

2 participants