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

Command to delete grids that fail an IsWorking check for a specified block. #166

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kontu
Copy link

@kontu kontu commented Aug 6, 2020

Commit 911b610 is just updating my fork to the current TorchAPI/Essentials master.

Commit 6aafed3 adds a "delete offtype" command to "!cleanup" to delete grids based on whether a specified blocktype is non-functional isWorking == false

Code originally written by Teknikal (AdamOckley), he just asked me to put in a clean pull request for it for him.

Copy link
Contributor

@LordTylus LordTylus left a comment

Choose a reason for hiding this comment

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

As commented an the file I would like that feature to be a condition instead of a separate command with duplicated deletion logic.

Unless Jim is fine with doing it that way.

@@ -59,6 +59,30 @@ public void Delete()
Log.Info($"Cleanup deleted {count} grids matching conditions {string.Join(", ", Context.Args)}");
}

[Command("delete offtype", "Deletes grids with specified blocks toggled off.")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a separate command tho?

I personally prefer it to keep it as !cleanup list or !cleanup delete

Instead you could have added a new condition you can see how for example HasType is implemented in line 385

As it is implemented now I have not the ability to just "list" which grids would be affected. And since I cannot see what would be affected I cannot check if my filter is correct or I for example need to exclude something with other conditions.

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