Skip to content
This repository has been archived by the owner on Mar 26, 2024. It is now read-only.

[WIP] Prune reports that match a specific status #378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZeroPointEnergy
Copy link
Collaborator

@ZeroPointEnergy ZeroPointEnergy commented Sep 11, 2018

This is a rebase of the PR #311 so we have a new working branch to test and discuss the new feature. I also squashed the changes into one single commit.

THIS WORK IN PROGRESS!
This PR is not tested and should not be merged currently.

@ZeroPointEnergy ZeroPointEnergy changed the title Added: Prune reports that match a specific status [WIP] Prune reports that match a specific status Sep 11, 2018
@sodabrew
Copy link
Owner

Two defects of the original PR, I never voiced my concerns though:

  1. The SQL changes effectively make these arguments mandatory. That's a non-starter; leaving out these arguments needs to function in the old behavior of nuking all reports past the given age. Clearing all reports beyond a given age would require at least two invocations in the current version of the code.

  2. I'm not a fan of the assembly of two arguments. I'd prefer a single argument, even if it's something like status=is-unchanged or status=not-pending that requires further parsing. That said, the pattern was set by upto and unit which assemble two arguments into the older-than date. I had always wanted to change this to something like age=30d or older-than=2m for example, but it's not exactly a high-value win to clarify these arguments.

@ZeroPointEnergy ZeroPointEnergy added this to the Release 3.2 milestone Sep 11, 2018
@ZeroPointEnergy
Copy link
Collaborator Author

Yeah this should better not be merged before we have some tests around the tasks to ensure the old behavior is still working. I added it to milestone 3.2 where I put all the other task related stuff. Maybe we can just use this as an inspiration or feature request when reworking the tasks so I leave it open for now.

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

Successfully merging this pull request may close these issues.

2 participants