Plugin addition - Submit Folksonomy Tags #349
Conversation
based on initial checks from codacy https://github.com/metabrainz/picard-plugins/runs/12546220957 I'm new to this, so... bleh.
Codacy really doesn't like the README.md (namely the list-item indicies), fixing it again https://github.com/metabrainz/picard-plugins/runs/12546981858
|
Only major issue is the complexity now. I should be able to work on that next week, if that's alright? |
That should be fine. I don't think there's any need to rush. The current working version is available from your repository if anybody wants to try it out, so I suggest that you take whatever time you need. For what it's worth, I still have a tendency to include everything in the same function / module / method myself. I think it's because I've done way too much "linear" programming in the past, and haven't totally embraced the concepts of OOP. 😁 One final thing, and this is really minor... I appreciate the credit that you've provided in the comments in your code, but I noticed that you referred to 'rswift' rather than 'rdswift'. Like I said, really minor. 😉 |
|
Alright, thank you! :) I've tested the plugin on Arch Linux (Python 3.10.10) / Picard 2.8.5 and all seems to be working just fine now. The complexity of the main function should still be sorted in the near future, but if there's no other problems I think it can be merged. |
There was a problem hiding this comment.
Thanks a lot for submitting this. Overall looks good to me and I had already tested your plugin a while back. Only one suggestion to use metadata.getall instead of splitting tag values.
The complexity of the main function should still be sorted in the near future, but if there's no other problems I think it can be merged.
Agreed. Factoring out some code into separate functions should do the trick, but I'm fine with merging this as is for now.
uses `xml.sax.saxutils` instead of `html` to provide XML escaping, and move said escaping into the function for generating XMLs instead of handling tags. I've also used `.getall()` for grabbing MBIDs as phw suggested.
|
I've started work on refactoring, and thus far I've managed to split the submission handler into different functions in a class. I'm using Radon as a way to check if I'm on the right track, will that pose an issue with Codacy? FWIW, the initial functions I have have a complexity of 13 at the highest (Radon also says B-grade) right now. |
You may know me as Victini on MusicBrainz or Flaky on Discord.
Put briefly, this plugin lets users submit tags from their files as MusicBrainz' folksonomy tags. I've gone into more detail here: https://community.metabrainz.org/t/submit-folksonomy-tags-plugin-for-submitting-tags-to-musicbrainz-via-picard/626105
The only major bug I have with the plugin is that submitting after logging in from Picard's "to do this you must login" popup gives me an Unauthorised error (any subsequent submits are fine and of course, logging on before submitting), but Aerozol suggested on Discord that I should submit this anyhow and fix it later.