This page serves mpdonadio's project application reviews and help to project application reviews. This is a part of mentoring to become a code review administrator eventually. Add any advice that you may have through comments.
Reviews. Some projects have multiple reviews; first is linked. I think this is chronological. #11 and on represent reviews done since the call to action.
- Token i18n Macrons
- Folder Menu
- Simple Pin Map
- Ubercart mbe4 Mobile Payment Method
- Simple Node Archive
- jQuery Scroll Follow
- Role memory limit
- MultiAnalytics
- Search Modify
- [D7] TAP CMS
- [D6][D7] Mobi2Go
- [D7] Commerce Cart Stats
- [D7] Commerce Payfirma Payment Module
- [D7] Trebutra - Trello Bug Tracker
- [D7] Commerce Google Tag Manager
- [D7] MyCharts module
- [D7] Ubercart Legal integration
- [D7] Sticky Sharrre Bar
- [D7] Context Flag - https://drupal.org/node/2213685#comment-8800299
- [D7] Nano Scrollbar - https://drupal.org/node/2220033#comment-8800411
- [D7] Video Embed Instagram - https://drupal.org/node/2212035#comment-8800763
- [D7] Commerce Moolah - https://drupal.org/node/2212461#comment-8801741
- [D7] Social Media Aggregator - https://drupal.org/node/2245645#comment-8801819
- Youtube Video Uploader - https://drupal.org/node/1874650#comment-8803161
- [D7] The City Group Map - https://drupal.org/node/2219823#comment-8804667
- [D7] Webform tracking - https://drupal.org/node/2220111#comment-8805245
- [D7] Custom Import - https://drupal.org/node/2222137#comment-8806845
- [D7] Age Verification Module - https://drupal.org/node/2013591#comment-8818543
- [D7] prettyPhoto Formatter - https://drupal.org/node/2215467#comment-8821327
- [D7] disable js - https://drupal.org/node/2223193#comment-8823057
- [D7] Semantic UI - https://drupal.org/node/2211003#comment-8823281
Comments
Welcome
Thanks for starting this, looking forward to your reviews! Feel free to ask questions here or in #drupal-codereview on IRC.
Any feedback as to what I
Any feedback as to what I have been doing well, and what I could improve on would be appreciated. Thanks.
Looking good
Your API knowledge is strong, you have a sense for detail and a good understanding of the code you are looking at. It looks like you found XSS problems in some cases, so bonus medal of security honour. Respect!
Project application reviews are basically sanity checks, so while providing all details what can be improved is very valuable we have to consider what issues are really blocking approval. If we are confident that the people basically know what they are doing then we should not hold them back. With the exception of security issues and licensing issues.
I personally set applications to needs work if the API usage is so painfully wrong or the spaghetti code issues just keep piling up. Fortunately people fix all the things from pareview.sh, so at least we don't have to complain about coding standards anymore.
I saw that you never mentioned licensing or project duplication issues in your reviews, maybe there were none. See also the project application checklist: https://drupal.org/node/1587704
Thank you!
Thanks, I really appreciate
Thanks, I really appreciate it. I'll work on better feedback about why something is a blocker, and be more explicit about licensing and duplication, especially when marking as RBTC.
I think it's time to promote
I think it's time to promote you to git admin now: https://drupal.org/node/2276911
Thanks
Thanks. I think I have a pretty good feel for what (codewise) is a blocking issue and what isn't now. The one thing I haven't totally gotten a hang of is when an application should be denied because of duplication (I haven't found any good examples yet). For example, what would the proper response be to these two: https://drupal.org/node/2235863 and https://drupal.org/node/2013591
No application should be
No application should be denied solely on the grounds of duplication. If people absolutely must duplicate efforts for whatever reason we should let them.
That said, we should really push hard our community spirit of collaboration over competition. That means we ask them to contact maintainers of existing projects first, open issues etc. If the maintainer rejects them, then we can continue the application. I demand that they at least document the other existing similar projects and the differences to them on their project page.
Promotion done,
Promotion done, congratulations! Feel free to ask me any questions that you might have when using your new powers: https://drupal.org/node/1125818 . You can find me in IRC in #drupal-codereview.