Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Project: https://www.drupal.org/sandbox/mikran/2371049
Translation importer module imports translation catalogs from a specified file stream to Drupal installations during automated deployments (eg. typical dev - staging - production workflows).
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/mikran/2371049.git translation_importer
Comments
Comment #1
PA robot CreditAttribution: PA robot commentedWe 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.
Comment #2
Uhkis CreditAttribution: Uhkis commentedAutomated Review
57 | WARNING | Line exceeds 80 characters; contains 86 characters
Only error is from a URL, shouldn't be a problem.
Manual Review
This review uses the Project Application Review Template.
Comment #3
mikran CreditAttribution: mikran commentedComment #4
PA robot CreditAttribution: PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxmikran2371049git
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #5
mikran CreditAttribution: mikran commentedComment #6
jribeiro CreditAttribution: jribeiro commentedHi Mikran,
The structure of the module looks really good! Congrats.
Manual Review:
1.
class TranslationImporterStreamWrapper extends DrupalLocalStreamWrapper {}
I don't know, in my opnion I think is always better isolate the class on a separated file, and list it on .info file. What do you think?
2.
Is a good practice, use the module name as a variable prefix to avoid conflict, like translation_importer_xxxxxxx.
So using the name of the module as a prefix, you can do something like this:
Just comments ;)
Automated Issues:
Comment #7
mikran CreditAttribution: mikran commentedThanks jribeiro for review. However, you didn't change the status of the issue. I believe none of the issues raised in manual reviews are blockers. I've added some code after the first review so the 120 loc issue shouldn't exist even if tests are not counted.
Comment #8
mikran CreditAttribution: mikran commentedComment #9
InviteReferrals CreditAttribution: InviteReferrals commentedhttp://pareview.sh/pareview/httpgitdrupalorgsandboxmikran2371049git
Comment #10
InviteReferrals CreditAttribution: InviteReferrals commentedComment #11
mikran CreditAttribution: mikran commentedThe mentioned whitespace issues are now fixed.
@InviteReferrals just posting a link to pareview.sh does not count as a review, you should also do a manual review of the code.
Comment #12
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedRTBC, looks great.
Guidelines are followed, found no security problems.
Comment #13
cweagansThanks for your contribution!
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.