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

Issue-379: Change clear-cache to clear-temp #382

Merged
merged 4 commits into from
Sep 17, 2024
Merged

Issue-379: Change clear-cache to clear-temp #382

merged 4 commits into from
Sep 17, 2024

Conversation

Edwin-Dijk
Copy link
Collaborator

Small change but decided I would get it done

@Edwin-Dijk Edwin-Dijk requested a review from tpill90 September 13, 2024 11:32
@Edwin-Dijk Edwin-Dijk marked this pull request as draft September 13, 2024 11:41
@Edwin-Dijk
Copy link
Collaborator Author

Edwin-Dijk commented Sep 13, 2024

Converted to draft quickly as the documentation also has to be updated
EDIT: now updated documentation aswell, @tpill90 let me know if the ClearCacheCommand class name needs changing, I personally would not do this as this is pretty much never seen by users?

@Edwin-Dijk Edwin-Dijk marked this pull request as ready for review September 13, 2024 11:50
@tpill90
Copy link
Owner

tpill90 commented Sep 13, 2024

Personally I'd prefer to keep the class name matching what the actual command name is despite users never seeing it. I'd prefer this because it can help with understanding the codebase, especially for someone who is looking to contribute.

@Edwin-Dijk Edwin-Dijk self-assigned this Sep 13, 2024
@Edwin-Dijk
Copy link
Collaborator Author

Apart from the 1 remark it looks to me (without compiling atm as I don't have the time, will tomorrow) as if all previous mentions to cached files are changed now, new sentences look understandable aswell!

@tpill90
Copy link
Owner

tpill90 commented Sep 17, 2024

I think I'm overall happy with where we landed on this. Going to merge this in. Thanks for the help!

@tpill90 tpill90 merged commit 4d2c8b5 into master Sep 17, 2024
4 checks passed
@tpill90 tpill90 deleted the Issue-379 branch September 17, 2024 15:23
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