schnatterer / musicbrainzws2-java

java binding for MusicBrainz XML Web Service/Version 2
GNU General Public License v3.0
15 stars 10 forks source link

Fix ResourceNotFoundException (Annotations), fix attribute: type-id warnings, add missing properties, add constructors #5

Closed Voidi closed 2 years ago

schnatterer commented 2 years ago

You did it 👍 Hoping to be able to look into this soon. Have to migrate CI to GH actions first.

schnatterer commented 2 years ago

OK, we now have CI again. :tada:

Whats your status? Is this still WIP or ready to review?

Could you describe how to test this PR?

Do you have any ideas how to fix the breaking "unit" tests? It fails because it's actually an integration test running against test.musicbrainz.org, which seems to return 502. As I dont have any more time, so I'd just ignore the tests. This project has gained so much tech debt, it hardly matters anymore :grimacing:

Voidi commented 2 years ago

I think i'm ready with this. As havent really worked with Test yet, i can't help with this. I agree with you about the technical debt, so maybe ignore it. The test server Web interface works, but no parts from public API.

schnatterer commented 2 years ago

@Voidi it took me a while to figure out - you didn't just fix #4, but also added some missing attributes and constructors. A more concise title and description would have saved me some time there.

I implemented a test to verify #4 (fetchesAnnotations()) - could you please add similar tests for the new attributes (added in c662bb9e)? You can use my test as a template. Just query an exemplary ReleaseGroup, Disc, Recording, Work or what ever is necessary and assert that the attributes you added are as expected.

These tests can also be seen as a kind of documentation on how to use the new features.