-
Notifications
You must be signed in to change notification settings - Fork 12
Country fallback using MusicBrainz API #79
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
base: master
Are you sure you want to change the base?
Conversation
|
P.S. I also updated gulp-minify-css to gulp-clean-css because gulp-minify has been deprecated and was giving me build errors |
|
Thanks for chipping in! Will hopefully have time to check it out soon |
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.
why did you delete this?
| * @param {String} countryName - Country name (e.g., "United States", "Sweden") | ||
| * @returns {Object|null} Country data object with id, name, etc., or null if not found | ||
| */ | ||
| api.convertCountryNameToCountry = function(countryName) { |
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.
does this need to be exported? Can it be moved to musicbrainz.js?
|
I reviewed on my phone but it was a challenge, but fwiw here's a preliminary review: At a glance it looks like some musicbrainz-specific code could be moved from api.js to musicbrainz.js for better separation. Also consider just having an interval checking if anything's in the queue and processing if there is, to not have to explicitly check after several actions. Since this project is a load of spaghetti as it is (and barely maintained), the less new code the better. Try to minimize the number of null-/type-checks to what's required to not crash and assume valid data otherwise (lol) i.e. check before passing args i guess? in the same vein, the error logging in index.html can be removed Will review properly later! |
Hello!
I love explr, thanks for creating this
I created this little solution for the artists with missing country tags. It uses the MusicBrainz API as a fallback to fetch country data when Last.fm fails
MusicBrainz has a 1 request/second rate limit so this is done in the background while the list is loaded in parallel with the Last.fm process. I also added a little status indicator in the list of artists without a country for those that the process is currently trying to fetch from MusicBrainz
You can see it working on my own Github IO page: https://reidosbares.github.io/explr/
I hope you will consider adding this to explr!
And if this works, and you like it, please consider giving my own Last.fm project a shout-out as well: https://github.com/reidosbares/charts-fm
Thanks!