[D8] Audio Embed Field

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxvilepickle2784301git

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Manual Review

Individual user account

Here is my feedback:

1. The README has minor grammar and spelling issues: in the summary, add a comma after the second and; under Installation, change "Install as usual, see" to "Install as usual as per"; under Customization, "You may overide" should be "You may override"; change "When you add fields, select the providers you want to use including Custom URL and Soundcloud. If using Soundcloud you need to obtain a client ID from the Soundcloud Developers site: https://developers.soundcloud.com/" to "When you add fields, select the providers you want to use, including Custom URL and Soundcloud. If using Soundcloud, you need to obtain a client ID from the Soundcloud Developers site: https://developers.soundcloud.com/". Also consider that "Custom URL" sounds kinda strange as a "provider", but maybe that's just me.
2. Your dependencies in README don't match your .info.yml file. (Out of curiousity, why does this module depend on image?) Also looks like you may require the colorbox library.
3. ProviderManagerInterface.php, ProviderPluginBase.php, ProviderPluginInterface.php, and a few other files could probably be slightly better documented. Personally, I'd like to see abundant documentation for each method introduced by the module, but the documentation is still better than most modules.

Overall analysis: the above are mostly nitpicks, so I think this module is ready to be promoted.

Automated Review

Kindly check the pareview.sh link, some trivial issue you can fix... to adhere to Drupal coding standard's
http://pareview.sh/pareview/httpsgitdrupalorgsandboxvilepickle2784301git

Manual Review

Individual user account

[Yes: Follows] the guidelines for individual user accounts.

No duplication

[Yes: Does not cause] module duplication and/or fragmentation.

Master Branch

[Yes: Follows ] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

3rd party assets/code

[Yes: Follows] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.]

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (*) Major finding, needs work
2. (+) Release blocker
3. Just a recommendation
4. ...]

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

@aditya_anurag: Looks like you forgot to change the status. Is this now RTBC after your review or are there application blockers left and this should be "needs work"?

@klausi & @vilepickle : I have updated the comment as per pareview.sh

Is it just me or is anyone else having problems to get this module to work with Soundcloud? I've registered an application, entered the client ID into settings page and tried to insert a link to a Soundcloud song into the field.

I always end up with this error:

Could not find an audio provider to handle the given URL. If you are trying to use something with an API, make sure credentials are set up at the Audio Embed Field settings page.

@vilepickle: It is actually the other way around as i would have expected it. When adding this field, the options below have a title called Allowed Providers. I was checking Soundcloud to ALLOW it but it was disabling it.

This is contradicting the title:
"Restrict users from entering information from the following providers. If none are selected any audio provider can be used."

I think you should either change the title or have checked providers to be allowed instead of disallowing.

Otherwise its working great!

There is a huge amount of code in here which is totally irrelevant to the module, left over from video_embed_field. The upgrade path from media_entity_embeddable_video to video_embed_media is a bit of a striking example. The whole thing looks like a crude string string replace on video => audio.

Given little original code was written -1 to granting full project access to the author. Perhaps with a lot of clean-up on the code itself it could be useful for servicing the audio-use case, but I doubt it in the current form. You could implement the VEF plugins in an audio-centric way and arrive at the same solution with 1% of the code.

Not sad at all, flattered if anything :)

I just don't think a find/replace qualifies for a project application and I think you could have taken the time to understand the code you were submitting and refine it.

even corrected a lot of problems with syntax in the orginal video_embed_field that overlaps the audio functionality since this review makes the submitter go above and beyond. I guess D.org doesn't hold module authors after they've past the "gate" to high standards anymore.

Can you open an issue in the video embed field queue so this can be fixed? Genuinely interested in what's going on here.

A process which doesn't burn out new contributors before they start is being discussed here: #2453587: [policy, no patch] Changes to the project application review process. Your frustrations are pretty much universal to people going through the queue.