-
Notifications
You must be signed in to change notification settings - Fork 277
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
Delete for replot #870
Open
graemes
wants to merge
5
commits into
ericaltendorf:development
Choose a base branch
from
graemes:delete_for_replot
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+33
−0
Open
Delete for replot #870
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
fb848ca
Add target def.n for replot single file removal
graemes 6496461
Changelog
graemes cbe5d6c
Correct double quotes (too helpful editor)
graemes e7c8032
Use site_root and change example
graemes db02d4e
Merge branch 'development' into delete_for_replot
graemes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this line the only change? Could it just be added to the existing targets with
replot_glob
being optional (and probably a different name) and execute only if specified? Is there a reason to usexargs
over command substitution?Not that this is a problem, but to make sure I understand, the expectation is to delete one plot from any archive drive and that the user will have consistent and separate directories across all archival drives for to-be-deleted plots vs. being-created plots. For example, I have
*/original/*.plot
that I will end up deleting and I am now plotting to*/pool.xch<contract address here>/*.plot
. So, myreplot_glob
would be*/original/*
. Also, in a decade when we all need k33 and our drives just refuse to die we can use this with theplot-k32-
as in your example and replace with k33. And in the end, the user just has to think about and be sure to setup the cross-archive-drive consistency or hazard deleting the plots they are creating etc.Just to write it out, not to say there's a problem, this does require the user to provide a single plot worth of free space prior to this mechanism "working". That does avoid the complexity of the space check having to account for a plot that could be deleted. This seems like a good thing to avoid and it comes at a very low cost, though it does need to be made clear.
Anyways, a good bit of text but mostly just me thinking out loud so you can make sure I'm understanding properly.
As always, thanks for the work and the contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've understood the intent precisely.
The additional 'deletion' line is the only extra - created a separate target for simplicity and was adapted from a comment in the plotting forum without too much consideration.
You're right - the user will need to be aware of what it is doing re: deleting initial plot and construction of the glob but as you've pointed out it is a fairly 'simple' solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is easy, and other solutions would probably be hard, I wouldn't push back on this. However, it seems far from intuitive, so I'd like it if everything that's been written up here explaining what's going on is converted into documentation somewhere nearby...?
As for longer-term solutions, I think there are a variety of plot-management tasks which would be cool to eventually support -- replacing OG plots with NFT plots, replacing plots that have won blocks with new plots, migrating from k32 to larger, "defragging" empty space on plot drives, migrating plots from specified drives elsewhere to enable them to be decommissioned, etc. This would really be a "plot management" rather than the "plotting management" system plotman has been, but it would need to interface closely with the traditional plotman.