pr fixing issue exception when artist sorted name has no last name pa… #367
Conversation
…rt issue#350 fixes metabrainz#350 added conditional , that recognises a single word artist name different from one with a last name .In such cases, the code generates initials for this single word to serve as an abbreviated representation.The initials are formed by taking the first letter of each part of the single word and appending a dot ('.') after each letter. moreover the code also handles the whitespaces in sorted and unsorted names .
There was a problem hiding this comment.
Sorry - but I am unable to review this because it contains multiple changes in a single commit and it is impossible to relate changes made to the problem they are fixing:
a. To fix the exception reported in #350;
b. To do other stuff
There are no examples that have been provided to test the use cases that have changed both before and after changes, nor examples that can be used to test that functionality has not changed for existing releases that work just fine.
In addition, there seems to be a for ... else statement that has been removed but I cannot determine why this was removed and what the impact would be.
| sortTag, | ||
| ) | ||
| log.debug("Abbreviated (%s, %s) to (%s, %s)." % (surname, forename, surname, inits)) | ||
| else: # while loop ended without a break i.e. no errors |
There was a problem hiding this comment.
Why did you remove this else statement? Removing this else statement changes the whole meaning of the statements that follow it.
|
okay , I'll take this into account and redraft smaller pr's in a systematic manner to make it easier to review and more step by step to resolve any issues in the code as well . in doing so , I'll also provide with the adequate testing use cases for both before and after the changes to assist with testing the changes in the code . thanks . |
|
@sambhavnoobcoder Yes, that would be great. I find this a bit difficult to review as it is, but you already had to wrap you head around all the functionality here. Especially some real test cases, if you have some, would be useful. |
|
I would love to see this get fixed, having quite a few errors with this plugin. |
…rt issue#350
fixes #350
added conditional , that recognises a single word artist name different from one with a last name .In such cases, the code generates initials for this single word to serve as an abbreviated representation.The initials are formed by taking the first letter of each part of the single word and appending a dot ('.') after each letter.
moreover the code also handles the whitespaces in sorted and unsorted names .