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

[DOCS]: Update unclear clean docs #475

Merged
merged 6 commits into from
Jan 29, 2024
Merged

Conversation

rvandernoort
Copy link
Contributor

Signed-by: Rover van der Noort s.r.vandernoort@student.tudelft.nl
Co-authored-by: Mark 16909269+Archmonger@users.noreply.github.com

Type of PR (feature, enhancement, bug fix, etc.)

Docs update

Description

Docs about clean were wrong and unclear, so I updated them with the docs from the code.

Fixes n.a.

Why should this be added

Explains the option better.

Checklist

  • Documentation has been added or amended for this feature / update

@Archmonger
Copy link
Contributor

Not a huge fan of the way the new description is written. Seems more confusing in a way.

@rvandernoort
Copy link
Contributor Author

Updated the sentence wording which was hard to read, is it better like this? I think this wording actually reflects what the feature does.

Copy link

codecov bot commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a8deb81) 91.22% compared to head (9b7cb15) 91.22%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #475   +/-   ##
=======================================
  Coverage   91.22%   91.22%           
=======================================
  Files          19       19           
  Lines         855      855           
  Branches      150      150           
=======================================
  Hits          780      780           
  Misses         40       40           
  Partials       35       35           

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

@Archmonger
Copy link
Contributor

Archmonger commented Jan 29, 2024

Revisiting this PR now that our CI workflows are functional again.

I think your comment related to the "first day of the month" might not be valid anymore. I do see that the docstring within dbbackup.management.commands._base.BaseDbBackupCommand._cleanup_old_backups does say we do that. However, the source code for dbbackup.storage.Storage.clean_old_backups doesn't appear to differentiate between "first of the month" backups, and any other backup.

This PR can be merged after removing that comment in the docs, and fixing the incorrect docstring.

@Archmonger Archmonger merged commit 1b2ead2 into jazzband:master Jan 29, 2024
9 checks passed
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