[D7] orgmode

welcome

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

Review of the master branch:

• README.txt is missing, see the guidelines for in-project documentation.
• Remove "version" from the ./orgmode.info file, it will be added by drupal.org packaging automatically.
• The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
  
  

  ./orgmode.module
  ./orgmode.inc
  

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

while waiting for an in-depht review of your module you can start out fixing some coding style issues detected by automated tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxdavidam1975296git

We do really need more hands in the application queue and highly recommend to get a review bonus so we will(/can) come back to your application sooner.

regards

Hi davidam,

• Please append to current issue link to Project page.
• It appears you are working in the "master" branch in git. You should really be working in a version specific branch. Please read how moving from a master branch to a version branch.

My manual code review:
orgmode.inc:

• Remove 12-13 line, it's duplicate 21,22 lines
• 22 line - why you not use ->fetch... method?
• 11 line - wrong parametrs list.
• 68 line - wrong comment * Submit handler for orgmode_form_validate(). for submit handler

Have a nice day!

minor change

I don't see a link to the project page here. Please add a link to this issue to make this easier for people to review.

Add file to tests

Is this functionality already present in the Feeds module? Is .org a file format? Perhaps you could use one of the Feeds Parsers, or add yours if it is unique.

There are also security problems in this module. You're pretty much assuming that the admin is the only person using the upload form, but the access check on that url is _node_add_access, which returns TRUE if the user has access to create nodes of any content type.

Please re-open this issue if you feel differently.

----
Top Shelf Modules - Enterprise modules from the community for the community.

kscheirer is correct about the security problem. Node create permissions are content-type specific, but you're not treating them that way. If I give anonymous users permission to create Article nodes, but not Basic Page nodes, your module is allowing them to create Basic Page nodes anyway. That's a clear security problem.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/2058247

Project 2: https://drupal.org/node/1977240

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

I've spoke to @davidam and he prefers this project application instead of the other one.

About the security issue:

The module should provide a permission like "can upload orgmode file", to be used as an access argument in the menu item.

Additionally, in the upload form, you must restrict available content types to the ones the user has access to. That is, check for "create $type content" permission on each of them.

OTOH, instead of the raw sql query on line 18 of orgmode.inc, you may want to use Drupal API: node_type_get_types().

---

On module structure and scope:

I think it would be of interest to isolate the parser from the rest of the code into a function / class. This way, the module could provide some useful interfaces to the parser:

* input filter: allow users to enter text in orgmode in a node body (or any textarea) and transform it into html on node view.
* feeds parser.
* perhaps some formatters...

Also, it could be easier in a future to change the parser by a external library, just in case.

add sandbox link

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

Couple of issues and warnings...

1. Change the git link on the issue page.

2. Remove the warnings from both and file.

Look clean. Good work!

Status RTBC does not automatucally give you the right to promote the project. Before that happens, one of the code review administrators will have to go through it one more for a final review. These are extremely busy people, so the queue of RTBC projects waiting to be "fixed" is long.

You just have to be patient.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. n/a

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

(+) No: Follows the guidelines for in-project documentation and the README Template. I don't really understand the purpose of the module. The README isn't clear about purpose. What is orgmode?

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No. If "no", list security issues identified. See issue below.

Coding style & Drupal API usage

1. (*) Not all nodes have a body field. You'll need to some logic to check fields on node types and filter by types that have either a body or equivalent field. Maybe make the destination field configurable?
2. (*) Uploaded files get stored in the public file system. This opens a security vulnerability to bypass security controls on nodes and view the original .org file. Either specifically call this out in the project documentation or default to storing the .org files in the private filesystem.
3. (+) hook_menu access callback defaults to user_access. No need to duplicate that functionality in orgmode_page_access
4. orgmode_form_upload_submit: Not all sites use sites/default/files as their public file storage. Use variable_get('file_public_path' conf_path() . '/files')
5. Probably should build a config page for this module so you can set defaults for status, promote, sticky, etc.
6. Why can't the author be the user that submits the org file?
   

   global $user
   $node->uid = $user->uid;
   

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.

Try the push a couple times. Sometimes it doesn't work the first time.

@davidam, thanks to you for contribution.

Manual Review

Coding style & Drupal API usage

1. (*) If user intentionally or unintentionally, upload a .org file with some invalid format, it produces following fatal error.
   
   
   
   
   It should be fixed during validation if file is not well formatted, either it prevent from uploading or display some meaningful message to user.
2. (+) You should create separate permission for admin/orgmode instead of using administer site configuration.
3. (+) If I upload another .org file, still it produces following notices every time.
   
   
   
4. (*) There is curly braces are mising in following query for table name.
   $field_name = db_query('select field_name from field_config_instance where bundle=:type and field_name=:field_name', array(':type' => $type, ':field_name' => variable_get('orgmode_bodyfield')))->fetchField();, look into db_query() once.
5. Please add your comment for following code how you are using this code? why do you need it?
   

   function orgmode_page_access() {
     if (user_access('view upload orgmode page')) {
       return TRUE;
     }
     return FALSE;
   }
   

6. (+) orgmode_form_settings: Why are you writing separate submit function as you are saving form values through varibale_set() that can be easily achieved through system_settings_form().

   Also you have not set default value for variable_get() for any of your form element, as it should be.

   Your code should be something like this:

   function orgmode_form_settings($form, $form_state) {
     $form['orgmode_published'] = array(
       // This is an HTML <input>.
       '#type' => 'checkbox',
       '#input' => TRUE,
       '#title' => t('Published'),
       '#default_value' => variable_get('orgmode_published', 0),
       // @todo: Explain #return_value.
       '#return_value' => TRUE,
       '#process' => array('form_process_checkbox', 'ajax_process_form'),
       // Use theme('form_element') to provide HTML wrappers for this element.
       '#theme' => array('checkbox'), 
       // Place the title after the element (to the right of the checkbox).
       // This attribute affects the behavior of theme_form_element().
       '#title_display' => 'after'
     );
   
     $form['orgmode_sticky'] = array(
       // This is an HTML <input>.
       '#type' => 'checkbox',
       '#input' => TRUE,
       '#title' => t('Sticky'),
       '#default_value' => variable_get('orgmode_sticky', 0),
       // @todo: Explain #return_value.
       '#return_value' => TRUE,
       '#process' => array('form_process_checkbox', 'ajax_process_form'),
       // Use theme('form_orgmode_checkbox') to render this element on output.
       '#theme' => array('checkbox'), 
       // Use theme('form_element') to provide HTML wrappers for this element.
       '#theme_wrappers' => array('form_element'),
   
       // Place the title after the element (to the right of the checkbox).
       // This attribute affects the behavior of theme_form_element().
       '#title_display' => 'after'
     );
   
   
     $form['orgmode_promote'] = array(
       // This is an HTML <input>.
       '#type' => 'checkbox',
       '#input' => TRUE,
       '#title' => t('Promote to front'),
       '#default_value' => variable_get('orgmode_promote', 0),
       // @todo: Explain #return_value.
       '#return_value' => TRUE,
       '#process' => array('form_process_checkbox', 'ajax_process_form'),
   
       // Use theme('form_example_checkbox') to render this element on output.
       '#theme' => array('checkbox'), 
       // Use theme('form_element') to provide HTML wrappers for this element.
       '#theme_wrappers' => array('form_element'),
   
       // Place the title after the element (to the right of the checkbox).
       // This attribute affects the behavior of theme_form_element().
       '#title_display' => 'after'
     );
   
     $form['orgmode_bodyfield'] = array(
       // This is an HTML <input>.
       '#type' => 'textfield',
       '#input' => TRUE,
       '#title' => t('Body Field'),
       '#default_value' => variable_get('orgmode_bodyfield', ''),
       '#return_value' => TRUE,
       '#process' => array('form_process_checkbox', 'ajax_process_form'),
       '#theme' => array('textfield'),
       '#theme_wrappers' => array('form_element'),
       '#title_display' => 'before'
     );
   
     return system_settings_form($form);
   }
   

7. (+) There should be .install file to delete all the variables that are set by this module after uninstalled of this module.
8. (+) orgmode_create_node: Instead of using time(), should use REQUEST_TIME.
9. (+) orgmode_form_upload_submit: this is also vulnerable to XSS. All data comes form user provided input should be sanitize before passing further, make sure to read https://www.drupal.org/node/28984 again.
10. hook_help() is missing in your module.

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.

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

As I am not a git administrator, so I would recommend you, please help to review other project applications to get a review bonus. This will put you on the high priority list, then git administrators will take a look at your project right away :-)

Thanks Again!

Please don't RTBC your own issues, see the workflow: https://www.drupal.org/node/532400

REQUEST_TIME is a static variable in that it represents the time the page request was made, not the current time. If there are several requests for the page request time (which may originate from different projects), calling time() from each of these projects may create race conditions with unpredictable results that are hard to debug (I know this the hard way).

Hence, there is a general agreement in the Drupal developer community that using REQUEST_TIME is the "right" way to do this.

In addition REQUEST_TIME is a simple lookup of a static variable, so it is fast. time() actually runs a syscall in order to check the time when this line is called, so it is slower, but that is a secondary consideration.

(*) There is curly braces are mising in following query for table name.
$field_name = db_query('select field_name from field_config_instance where bundle=:type and field_name=:field_name', array(':type' => $type, ':field_name' => variable_get('orgmode_bodyfield')))->fetchField();, look into db_query() once.

These curly braces let the PDO statement work with table prefixing, typically for multi-site installs. See https://api.drupal.org/api/drupal/includes!database!database.inc/group/d...

Hello,

Thanks for your module.

Pareview.sh, there are points to fix : http://pareview.sh/pareview/httpgitdrupalorgsandboxdavidam1975296git

Test in a Drupal environment with your test file : Thanks for test file, really useful when you don't know the orgmode syntax. In the h2 : I have "\" added, example : db\_insert, is it normal ?

Manual review :

orgmode.install :

• I see 3 variable_del, but there are 4 form elements in admin/orgmode, is it normal ?
• missing doc : Implements hook_uninstall().

orgmode.inc :

• Thanks, I didn't know this possibility of theming for checkbox.
• In the upload file form validate, is it necessary to check user_access('view upload orgmode page'), as it is in the hook_menu ?
• $field_name = db_query : why not using a db_select ? as you want.

Suggestions, very subjective :

• I think you can use your .org file in automated tests.
• I think a better position for admin/orgmode, would be admin/config/orgmode

Thanks for your module.

Hello,

I got a feature idea for your module. As I don't know orgmode, do you think it would be possible to add links on nodes (like the "add a comment" link on teasers) to get an orgmode version of the node ?

So your module would be able to import and export content from/to orgmode files.

That's just an idea I got, I don't know if it is possible and it is independant of the review application process. I wonder if I should not open an issue on the module instead of posting it here.

As I earlier mentioned regarding following point, you replied "On this way, the setting is not stored in the variable".

(+) orgmode_form_settings: Why are you writing separate submit function as you are saving form values through varibale_set() that can be easily achieved through system_settings_form().
Also you have not set default value for variable_get() for any of your form element, as it should be.

I have tested this code and it is storing values as required, you just replace your setting form with this code it would work for you. It would help to reduce extra submit function from your module.

Some more minor suggestions:

1. Last time I missed it, Instead of writing following custom query to check whether field exist or not, use field_info_instance.
   $field_name = db_query('select field_name from {field_config_instance} where bundle=:type and field_name=:field_name', array(':type' => $type, ':field_name' => variable_get('orgmode_bodyfield')))->fetchField();
2. (+) Currently you are using following validation to check whether title exist in file or not but it display message in green that dosn't make sense for user rather it should come in red.
   

    if (empty($title)) {
       return t('The title is mandatory.');
     }
   

   
   

   It should be:

   if (empty($title)) {
       form_set_error('file', 'The title in uploaded file is mandatory.');
     }
   

   
   

Thankyou for your patience and hard work. I hope after these fixes, it would be ready for RTBC.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. Yes, few minor issues, reported by http://pareview.sh/pareview/httpgitdrupalorgsandboxdavidam1975296git.

FILE: /var/www/drupal-7-pareview/pareview_temp/orgmode.inc
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------
253 | ERROR | If the line declaring an array spans longer than 80 characters,
| | each element should be broken into its own line
253 | ERROR | There should be no white space before a closing ")"
--------------------------------------------------------------------------------

Manual Review

Just few minor suggestions.

1. Now there is no use of following submit function as it is already override by system_settings_form(). Please remove this. system_settings_form() already produces The configuration options have been saved.message on successful submission.
   

    /**
    * Form validation handler for orgmode_form_settings_submit().
    */
   function orgmode_form_settings_submit($form, &$form_state) {
     drupal_set_message(t('Settings saved.'));
   }
   

2. In .info file, there is no need to use inverted comas, read https://www.drupal.org/node/542202 again.
3. orgmode_form_settings(): No default value is set for form elements as I have shown above in my code snippet.

4. Last time I missed it, Instead of writing following custom query to check whether field exist or not, use field_info_instance.
   $field_name = db_query('select field_name from {field_config_instance} where bundle=:type and field_name=:field_name', array(':type' => $type, ':field_name' => variable_get('orgmode_bodyfield')))->fetchField();

5. You can configure settings form link through .info file to make it directly accessible from module page.

I tested module functionality node get created successfully if .org file is valid.

My blocking issues from #36 and #48 have been addressed. Good Job!

I personally feel this is good to go now. I am not seeing any additional blocking issues, so I am setting RTBC. As I am not a git admin, so it needs a second set of eyes from a review admin.

Thank you for your patience and hard work davidam!

PS: Getting Review bonus would help to speedup your module promotion.

Don't forget to add the "PAReview: review bonus" tag, once you are done with at least 3 manual reviews as indicated in #1410826: [META] Review bonus, otherwise you won't show up on high priority list.

After you did at least 3 reviews of other applications you have to reference them in the issue summary of your application issue. You can edit the first post (=issue summary) of an issue. Create a new section "Reviews of other projects" and add links to the exact review comment, e.g. like this one http://drupal.org/node/1381726#comment-5418942

Having an admin review it to get the project promoted can take some time. I saw that you have one manual reviews done, other three are automated review report (removed). Two more manual review and you can add the PAReview: review bonus tag :).

Please note that the requirements for passing review laid down by this checklist: https://www.drupal.org/node/1587704. Please review item 4.1.

According to #30, your project was fine regarding licensing two months ago. However, after that you added LICENSE.txt to the repo.

License

(+) Please remove the LICENSE.txt file. Drupal will add the appropriate version automatically during packaging so your repository should not include it.

Since the license file just specifies GPLv2+ this is not an application blocker. Please remove it nevertheless.

manual review:

1. orgmode_form_upload_validate(): The form API will make sure that only people can submit this form that have access to the form, so you can remove the user_access() check.
2. orgmode_form_upload_submit(): do not hard code "sites/default/files" here since that might be in other places on Drupal installations. Use file('public://' . $file->filename); instead.
3. orgmode_form_upload_submit(): the check_plain()s are wrong here, since you are not printing the variables to HTML to the user. "When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database" from https://www.drupal.org/node/28984
4. orgmode_form_settings(): doc block is wrong, this is not a hook. See https://www.drupal.org/coding-standards/docs#forms
5. "$node->language = 'und';": do not use "und", use LANGUAGE_NONE instead.
6. orgmode_create_node(): do not use check_markup() here, since you are not printing anything to the user. Same as mentioned before: content should go unaltered to the database.
7. orgmode_create_node(): You should not throw form validation errors here, that should be done in the form validation callback orgmode_form_upload_validate().
8. orgmode_create_node(): so $field_name is never actually used and everything is hardcoded to the body field?
9. orgmode_create_node(): the text formats "filtered_html" and "full_html" might not exist on every drupal site and should not be hardcoded here. I think you should provide the text format as parameter in the upload form, where you only provide the options a user has access to with filter_access().
10. orgmode_create_node(): so full_html means a user with the permission 'view upload orgmode page' can upload arbitrary HTML and do XSS attacks. So either that permission needs to be marked as 'restrict access' => TRUE or you need to follow my proposed approach to restrict the filter formats the user has access to. Currently this is an access bypass vulnerability since a user can create full_html nodes even if they don't have access to that format (and a security blocker).
11. orgmode_form_settings(): why do you need the #theme keys here if you just use the default theme function? Can't you just omit them and the form would be the same?

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to myself for next review, which will hopefully be tonight.

Automated Review

(+) Best practice issues identified by pareview.sh / drupalcs / coder. Yes

Review of the 7.x-1.x branch (commit 7604a0d):

FILE: /var/www/drupal-7-pareview/pareview_temp/orgmode.inc
--------------------------------------------------------------------------------
FOUND 17 ERRORS AND 1 WARNING AFFECTING 16 LINES
--------------------------------------------------------------------------------
47 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
48 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
49 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
50 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 6
51 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 6
52 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 8
53 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 8
55 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 6
56 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 8
59 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
60 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 6
75 | ERROR | [x] Whitespace found at end of line
136 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
136 | ERROR | [ ] Doc comment short description must end with a full stop
137 | ERROR | [x] Expected 1 space(s) before asterisk; 0 found
137 | ERROR | [ ] There must be no blank lines after the function comment
208 | WARNING | [ ] Line exceeds 80 characters; contains 89 characters
243 | ERROR | [ ] Files must end in a single new line character
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 14 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Manual Review

Some of the issues mentioned in #64 still pending that need to be address.

1. (+)
   

   do not use check_markup() here, since you are not printing anything to the user. Same as mentioned before: content should go unaltered to the database.

   I'm printing the body and the summary. It checks variables for a drupal node..

   No, you are not printing anything in orgmode_create_node() function, you are just programmatically creating node over there. When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database.

   See https://www.drupal.org/node/28984

2. (+)
   

   You should not throw form validation errors here, that should be done in the form validation callback orgmode_form_upload_validate().

   Are you sure? The validation errors here is from parser, not errors from form.

   Yes, you can validate this in form validation callback function too by adding one more option of content type field in admin/config/content/orgmode form. It would also help to ensure whether given field exist in selected content type or not. One more thing, following code will fire only if the field name doesn't exist in selected content type. In that case $field_name always be blank so message appearing on screen is given below that doesn't make any sense because sentence is not completed.

   This content type (article) hasn't a tested_body field and field_name is .

3. (*)
   

   orgmode_create_node(): so full_html means a user with the permission 'view upload orgmode page' can upload arbitrary HTML and do XSS attacks. So either that permission needs to be marked as 'restrict access' => TRUE or you need to follow my proposed approach to restrict the filter formats the user has access to. Currently this is an access bypass vulnerability since a user can create full_html nodes even if they don't have access to that format (and a security blocker).

   Issue still exists.

4. (+) orgmode_form_settings() : What is the use of orgmode_css form element here. Comment Needed. Make sure all variables defined by your module need to be removed in hook_uninstall(). Currently orgmode_css is not removed in hook_uninstall(). See https://api.drupal.org/api/drupal/modules%21system%21system.api.php/func...
5. (+) orgmode_form_settings(): in every form element '#process' => array('form_process_checkbox', 'ajax_process_form'), appearing, even you are not processing any action here. It should be remove as per current scenario. See https://api.drupal.org/api/drupal/developer%21topics%21forms_api_referen... again.

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.

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

Review of the 7.x-1.x branch (commit b529250):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/orgmode.inc
  --------------------------------------------------------------------------------
  FOUND 7 ERRORS AND 1 WARNING AFFECTING 6 LINES
  --------------------------------------------------------------------------------
    75 | ERROR   | [x] Whitespace found at end of line
   136 | ERROR   | [x] Expected 1 space(s) before asterisk; 0 found
   136 | ERROR   | [ ] Doc comment short description must end with a full stop
   137 | ERROR   | [x] Expected 1 space(s) before asterisk; 0 found
   137 | ERROR   | [ ] There must be no blank lines after the function comment
   181 | ERROR   | [ ] Inline comments must end in full-stops, exclamation marks,
       |         |     or question marks
   205 | WARNING | [ ] Line exceeds 80 characters; contains 89 characters
   239 | ERROR   | [x] Expected 1 newline at end of file; 2 found
  --------------------------------------------------------------------------------
  PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
  --------------------------------------------------------------------------------
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. project page: what is orgmode? Is that a file format? What is the use case, why would you use this module? See https://www.drupal.org/node/997024
2. why is there a README.org file? Ah, is that the special file format you are referring?
3. "db_query('select field_name from {field ...": please follow the SQL query coding standards: https://www.drupal.org/node/2497
4. orgmode_create_node(): why do you need the DB query anyway? You should use field_info_field() instead.
5. The orgmode_css variable is currently unused? Please add a comment.
6. "$node->body[$node->language][0]['format'] = 'filtered_html';": so the filtered_html text format is now hard-coded, which might not exist on every site. I think you should add a text format selection in orgmode_form_upload() so that the user can select a format they have access to.

But that are not application blockers, otherwise I think this is RTBC.

Assigning to stBorchert as he might have time to take a final look at this.

My additions to the review of klausi:

* Path to upload form: in my opinion this would be better something like admin/content/orgmode (or similar) instead of simply orgmode
* Configuration form: while first looking at the form I didn't have any clue what the options are for. Some descriptions and help would be great here. I still have no idea what the CSS path is used for ;)
* Import form: wouldn't it be much better to include the options from the configuration form in this form? Say I have different content types where I would like to import org-files into. Every content type has a different structure and a different field for its content (we prefix non-shared fields with the content type name so this could be something like field_article_content or field_landingpage_heading). Using one global field name for the import destination would force the site builder to use the same field in all content types he wishes to import org-files into.
* Import form: using the human readable names in the content type selection would be much better. Oh, while writing about it: you could provide another dropdown for the destination field which automatically updates on content type selection. This would be a really nice feature ;)
* File storage: as klausi said before it's not always good to store the files in the public file system. Maybe you could add a configuration option for this.

But as these are no applications blockers 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:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.