Comments

• 

  Plugins bot

  Wed, 27 Apr 2016, 2:42 PM

  Prechecker results: 306 errors/14 warnings
  http://integration.moodle.org/job/Precheck%20remote%20branch/23121/artifact/work/smurf.html
• 

  Dan Marsden

  Fri, 29 Apr 2016, 5:24 PM

  Hi Antonello,
  
  As a member of the plugins guardian team I'm taking a look at your plugin. Sounds like an interesting idea!
  
  It would be good if you could expand the description here to list some possible use-cases and examples of how the plugin can be used so that it is clear what your plugin does - many people reading your plugin will not understand the terms "Sql query" or "dynamic".
  
  Your github repository appears to contain your IDE config/environment setup files at the root of the repository - this is not considered good practice and the plugin files should sit at the root instead - for example:
  https://github.com/Microsoft/moodle-profilefield_o365
  
  You may also want to consider adding travis-ci support to your github repository allowing the various coding checks like the ones shown by the prechecker above to be run automatically on your code:
  https://moodle.org/mod/forum/discuss.php?d=323384
  
  I will take a look at your code and try to give further feedback within the github tracker.
  
  thanks,
• 

  Dan Marsden

  Fri, 29 Apr 2016, 6:12 PM

  Hi Antonello,
  
  I managed to get your plugin working and I can see some places where it could be quite useful - great work!
  
  Initially I was curious why you were storing the "key/id" value in the field instead of the actual text/data from the query? - I assume it is so that you can allow the data field to be updated in the external table and not need to modify the data stored in the field to match. The drawback with this is that each time the field is displayed it requires the sql to be called and in some cases the profilefield constructor is called multiple times on a single page load which then makes your sql run multiple times as well. I've reported some information about that in github but you should make sure that your use-cases/examples and description in the plugins entry describes that you have implemented it with an id/data field to allow the data field to be updated.
  
  for now we will mark your plugin as requiring more work - please let us know when you have had a chance to look at the issues found and upload a new version for us to review.
  
  thanks!
• 

  Antonello Moro

  Fri, 29 Apr 2016, 6:29 PM

  Hi Dan,
  Thanks for your help! I'll do my best to fix everything as soon as possible!