The ODataProducer module provides integration with the ODataProducer for PHP.

Project page: http://drupal.org/sandbox/daeron/1920596
Repository: git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/daeron/1920596.git odataproducer
Module is for Drupal 7

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Welcome,

please get a review bonus first. Then try to fix issues raised by automated review tools: http://ventral.org/pareview/httpgitdrupalorgsandboxdaeron1920596git

freblasty’s picture

freblasty commented
Status: Needs work » Needs review

Ventral issues have been resolved.

jwjoshuawalker’s picture

jwjoshuawalker commented

Ventral is clean.

Project page is a little empty, is there anything else you can mention about the module? Even if it's the same URL to that github page, put that in as the "Documentation" URL.

http://drupal.org/node/997024

sreynen’s picture

sreynen commented
Title: ODataProducer » [D7] ODataProducer
chason’s picture

chason commented
Title: [D7] ODataProducer » ODataProducer
Status: Needs review » Reviewed & tested by the community

Manual review:
@freblasty - looks like there is another project similar to yours here - http://drupal.org/sandbox/r2integrated/1561302 . Can you give an idea of how your module would be used in a real-world scenario? It would be helpful to know if you have a different vision for yours than the OData Server module by r2integrated.

I installed your module, ODataProducer, in D 7.19. It installed successfully - no errors. I would consider changing the path to the ODataProducer library in your README.txt file from "lib/ODataProducer" to "library/ODataProducer" so someone might not think they're missing a directory when they download the ODataProducer library.

sreynen’s picture

sreynen commented
Title: ODataProducer » [D7] ODataProducer
freblasty’s picture

freblasty commented

@chason - thank you for reviewing my module.

Vision

The ODataProducer module should be used when a module wants to access the ODataProducer library for integration purposes. Its sole use is to allow other modules to access the ODataProducer library and provides auto class loading which is compatible with Drupal. By using a separate module we prevent other modules from having to write there own ODataProducer library class loading implementation so that they can focus on the integration. In other words you could see the ODataProducer module as single point of contact to the ODataProducer library.

Real-world scenario

When a developers wants to provide an OData integration he/she has only a few options:

  • Search for an OData module which provides the functionality he/she needs and modify it were needed
  • Or, write the full integration including how to integrate the ODataProducer library in Drupal.

Instead of having different developers providing different implementations on how to access the ODataProducer library, we now have a unified way of accessing it thus prevent compatibility issues with future OData integration modules.

In the case of the OData Server module, it should only provide the implementation and not define how the ODataProducer library is accessed in Drupal.

freblasty’s picture

freblasty commented

@chason - the README.txt file has been updated.
@drastik - project page has been updated.

patrickd’s picture

patrickd commented
Assigned: Unassigned » patrickd
patrickd’s picture

patrickd commented
Status: Reviewed & tested by the community » Fixed

Please make sure to set your default project branch to 7.x-1.x

Your project page is very informative, but your readme could use some more information. Maybe you'd like to try to keep them in-sync?

"package = OData": Are you sure that you need your own package namespace? Do you want to have modules implementing your API's using this package? Otherwise I'd recommend to remove the package specification.

You are trying to use watchdog() in hook boot - the problem here is that if the site uses dblog: The database will probably not be available on that early stage, I'd suggest to not use watchdog() there.
Maybe also instead of using the drupal_set_message(.. 'error') - maybe instead check this requirement in hook_requirement on 'runtime' phase?

Beside that it all looks pretty straight forward.

Thanks for your contribution!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.