This module provides Canpar shipping quotes for Drupal Commerce. It supports both the Canpar base rates and your shipper account-specific custom rates.

Based on Canpar Shipping Quotes developed by Tim Rohaly.

Requirements

  • Commerce Shipping 2.x
  • Commerce Physical Product 1.x

Sandbox project page

https://www.drupal.org/sandbox/oleksiyshevchuk/2409023

Git Clone Command

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/OleksiyShevchuk/2409023.git commerce_canpar
cd commerce_canpar

Manual reviews of other projects

Comments

PA robot’s picture

Status: Needs review » Needs work

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

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.

Timusan’s picture

Hi Oleksiy

An automated review (seen here) shows a few (maybe) show-stopping issues:

  1. There is a LICENSE.txt file included, as per Drupal licensing guidelines this has to be removed.
  2. There seems to be some cursing against Drupal's coding standards (about 30+ errors). It is best to fix the major ones as adhering to these standards make for consist module code so other developers can easily step in and the quality of the code is somewhat protected. See the automated review link for a full list of the errors.

Manual review:

It may also be of use to include a hook_help() in your commerce_canpar.module file that refers to your written README.md inside of Drupal (the module overview page). See here for more details.

I do not have a decent Commerce setup to fully test your module, yet it installs correctly without any errors. The shipping method is fully available in the shipping UI and it is configurable, the options are saved correctly.

me-great’s picture

Issue summary: View changes
InternetDevels’s picture

Status: Needs work » Reviewed & tested by the community

Automated review (link) shows us that default branch was selected and code standard errors were fixed.

LICENSE.txt file has been also removed.

Didn't see errors during module install config, shipping services and methods were created successfully, admin pages also work correctly

Good to go

me-great’s picture

Issue summary: View changes
me-great’s picture

Issue summary: View changes
me-great’s picture

Issue summary: View changes
Issue tags: +PAreview: review bonus
me-great’s picture

Issue summary: View changes
klausi’s picture

Status: Reviewed & tested by the community » Fixed

Thank you for your reviews, make sure to set the status to RTBC if you did not find blocking problems with the project.

manual review:

  1. "variable_get('commerce_canpar_weight_markup', '0'),": all variables defined by your module must be removed in hook_uninstall().
  2. commerce_canpar_service_rate_order(): the watchdog() call is not really helpful. You should log the error status/message from the service.

But otherwise looks good to me, so ...

Thanks for your contribution, Oleksiy Shevchuk!

I updated your account so you can 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 stay 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.

TR’s picture

Priority: Normal » Major
Status: Fixed » Active

I know this is open source and all, but copying large parts of my uc_canpar module without alteration, removing my name from the authorship block, and failing to acknowledge my work is NOT OK. You even have an identical copy of my Simpletest routines.

You haven't even contributed back any fixes/improvement to my module, assuming you found some things that needed to be fixed or updated.

Please correct this oversight - this module is largely a product of MY work, and I deserve to be acknowledged for it.

klausi’s picture

Category: Feature request » Task
Status: Active » Fixed

Oops, that is unfortunate. @Oleksiy Shevchuk: can you add proper credits to the README file where you got the original code from?

I think there is nothing left to do in the application queue here, so I would suggest that you open an issue in the module's queue and escalate that to drupal.org webmasters accordingly if you don't hear back from Oleksiy.

TR’s picture

Hah, the README.txt is a total copy of my README as well, with Ubercart changed to Commerce.

The thing is, he actually *removed* the @author tag in the code - that was the one thing he didn't copy. And on the project page he gives credit for the module to his current employer yet fails to mention me.

@klausi, you're comfortable granting this guy git privileges based on him copying someone else's work and taking credit for it?

klausi’s picture

I'm a bit uncomfortable, but as long as people produce useful modules I think we should let them. We could consider if there is enough original code written on top of the copied code and only do a single project promotion without giving the git vetted user role away. On the other hand people will always copy parts of well-written code when they need it, and that is actually a good thing ... so I'm not sure how strict we should be about that.

The missing credits are definitely disturbing our community spirit a bit, but they are also not application blockers. I would like to give @Oleksiy Shevchuk the benefit of the doubt and I'm sure he is going to fix that. New contributors might not be aware of all things, so we show them. If that does not work we can always escalate later :-)

jibran’s picture

Well I can completely understand @TR's reservations. I, also, started PA approval process by copying the existing module #1483130: User Reference URL Widget. But, before starting the module I contacted the original author @quicksketch and when he said to go ahead with the new module because he didn't want to do it at all then I started my thread and told him about the thread as well. I also added his name in credits and mentioned the original project on the project page. My readme is also a copy of original project.

I have to go through my own learning curve to understand and copy the original module. I feel proud about creating that module because that was my first real contribution towards the community. I think as a developer I'm honored that I re-wrote @quicksketch module who is imo is one of the best contib module maintainer.

Respectfully, if I were @TR I'd feel proud that the code I wrote was worth copying and I helped someone to contribute towards the community. For me it's not always about the name sometimes it's about the feeling and I think this is an awesome feeling to have ;-)

Once again I do understand @TR's point and I'm not trying to be disrespectful to anyone in any way. And, I'd also like to say let's not rob @Oleksiy Shevchuk from his moment after his hard work and efforts to get through this painful lengthy process of PA approval(I know that too because I had to go through that twice:) #1925466: [D7] Views fieldset style plugin).

tim.plunkett’s picture

A credit should go on the project page.
I don't agree with @author tags in drupal modules anyway, I've never once written a large piece of code without *some* help from someone else...

naveenvalecha’s picture

I think we should give credit to @TR by mentioning on the project page and it should be a proud feeling for @TR that someone is using his written code.We are respectful for all the community members and their contributions.So is it fine for you to mention your name on project page @TR ? Finally, Thanks for the writing reusable code.

me-great’s picture

@TR, @klausi I completely agree that it was a mistake not to add reference to TR's project. Sorry for that.
I will add link to uc_canpar and TR's profile to project's description, README.txt and bring back information about original code author to module file.

Regarding code, as far as I know the main idea of GPL license is to give everybody permissions to extend any code. Of course with keeping information about original authors.

Does that work for you?

me-great’s picture

Issue summary: View changes
me-great’s picture

Promoted full project as I didn't receive other claims.

@TR
Tim, sorry again, I didn't want to offend you. Thanks for pointing at my mistake

klausi’s picture

Thanks Oleksiy!

Status: Fixed » Closed (fixed)

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