Implement GoodJob.cleanup_preserved_jobs, fixes #351
#356
Conversation
aried3r
force-pushed
the
ar/implement_good_job_cleanup_preserved_jobs
branch
from
38fb393
to
23ff392
Compare
lib/good_job.rb
Outdated
| # delete old records and preserve space in your database. | ||
| # @params before_seconds_ago [nil,Numeric] Jobs olders than this will be deleted (default: +86400+). | ||
| # @return [Integer] Number of jobs that were deleted. | ||
| def self.cleanup_preserved_jobs(before_seconds_ago: nil) |
There was a problem hiding this comment.
I'm curious if you think it would be good to have a better kwarg name here. I want to suggest something like
cleanup_preserved_jobs(before: 3.days.ago)orcleanup_preserved_jobs(older_than: 3.days)<-- I think I like this one.- Something better?
The longer option names were really a result of wanting the CLI interface to be clear given that there are limitations in what can be passed through ENV. But as a Ruby method, it's easy to pass in a timestamp, or duration.
It's a kwarg so it's trivial to preserve both options, and there is a benefit to being symetrical with the CLI options, but I recognize it's a little clunky.
Not a blocker of this PR, I wanted to get your perspective :D
There was a problem hiding this comment.
I'm curious if you think it would be good to have a better kwarg name here.
Definitely!
When writing this, I was also considering older_than but wanted to stick with the CLI option name to avoid having to discuss a potential other name. So I'm very happy you brought it up!
I don't think we necessarily need to keep both names, but I'm open to that. I'll work on the changes and you can have another look if you want to keep this symmetrical after all.
aried3r
force-pushed
the
ar/implement_good_job_cleanup_preserved_jobs
branch
from
113b979
to
daf8e83
Compare
Co-authored-by: Ben Sheldon [he/him] <[email protected]>
|
@aried3r Thank you! 🎉 Released in |
No description provided.