manolomartinez / greg

A command-line podcast aggregator
GNU General Public License v3.0
296 stars 37 forks source link

Tagging subprocess is easy to break #124

Open n8willis opened 2 years ago

n8willis commented 2 years ago

For future reference, when updating greg, this commit: a2ffebadaac75dda5e2dfb002601ef1bbaedfb38 (which changed tag_comment to tag_comments) broke all of my tagging --- i.e., not just the "comment" tag --- and all (wget-based) file renaming in every feed: none of the tags were written and none of the downloaded files were renamed.

I assume this was because tag_comment had been already set in my local greg.conf.

I think that it's fairly common for users to have the tag_foos set, since they're in the example configs; it thus kind of causes unnecessary harm when a commit breaks things that are valid in existing config files. So, changing a valid tag_foo into an invalid one is inviting trouble; if there are other such changes in subsequent development, I think marking the old one as deprecated would be the way to go. Also, some higher-profile notifying of the change would be helpful (although I believe that goes hand-in-hand with rolling a whole new update release, something I'm still very much in favor of!)

In the future, it would also be best to not have the whole tagging apparatus die if it encounters a mismatch in the process of collecting to tags-to-be-applied. I have no opinion on how best to implement that.

It would also be good if the process raised an exception or reported an error. By silently failing, many many months worth of downloads went untagged. If an error had been shown somehow, I might have caught it.

(side note: why this would break file renaming for wget -O I can't say; that may be purely coincidental or due to some other commit merged in at the same time. I haven't bisected/drilled-down enough to identify the problem there yet, and will open a separate issue for it if I figure something out. But it did begin at the same time.)