Note:

If you want to create a new page for developers, you should create it on the Moodle Developer Resource site.

Talk:Modules and plugins replacement proposal

From MoodleDocs
Revision as of 17:14, 2 February 2011 by Joseph Rézeau (talk | contribs) (→‎typo?: new section)

Some minor suggestions from Tim (so much for the request not to comment yet):

  • This may be a version 2.0 feature, but simple links from the database entries to the place on AMOS where people need to go to translate this plugin, and back again.
  • In terms of which version of core Moodle the plugins works with, there are big advantages if this information can remain in version.php, where it is readable by the Moodle install code. The MnP database could automatically read the data out of there to display on the web, like AMOS gets strings out of version control.

In the database schema:

  • Rather than separate local_contrib_plugin_contributor.creator/active columns, would it be better to have a single contributortype column? Easier to extend in future.
  • Should userid be in local_contrib_review or local_contrib_review_outcome?
  • If there a good reason for using 'softwareversion' in names? moodleversion would make it clearer. Of course, that is less generic.
  • should local_contrib_review_outcomes have a machine-readable column that holds a status like pass/fail, as well as the text in review?
  • Do you really need hierarchical categories. They are a pain in SQL, and they add UI complexity. I think one-level categories may be fine, particularly if you have tags as well.

In terms of display, it would be nice if you could use group icons from using Moodle (like core-developer, Partner, PHM) when displaying users. It would be useful if people could see that a review was by a core developer.

I had not realised that the file API did not support local plugins. See https://github.com/moodle/moodle/blob/master/pluginfile.php#L738

Would a capability like "new entries from this user approved automatically" and "new versions from this user approved automatically" be worthwhile? I believe the current system overloads the approve capability for this, which causes pain.

--Tim Hunt 09:17, 12 January 2011 (UTC)

Hi Tim, Thanks for the early feedback, all good suggestions/observations. In response to the points you have raised:

  • AMOS links make VERY good sense and I'm sure I'll be able to easily fit them in.
  • Good suggestion, I still have to talk to Martin about how things will be stored, however where practical if possible it would be great if we could utilise version.php
  • Interesting idea, I had separated the two as the creator may no longer be active (passed the responsibility onto others). Perhaps if active is made contributortype that would allow us to extend in the future (primary or owner being an obvious extension).
  • userid I think should be local_contrib_review, there will be a record created there for each individual review which will consist of the general review stored within that record, and then several criteria reviews stored in local_contrib_review_outcome. Does that make sense? Remembering of course this is all very fresh and it may be me misunderstanding something here.
  • Yes, softwareversion isn't the greatest I did refrain from calling it moodleversion to keep the project more easily diversable, however I have yet to talk to Martin about that prospect... not too sure what he will want but I'll leave that one up to him.
  • In regards to local_contrib_review_outcome I was intending to you the core rating system for that, however it would require core changes presently. If the rating system isn't used then it will certainly need that column. Also I will have to at some point consider performance, I haven't looked at the ratings internal but if it isn't joinable in a friendly way then it may be worth duplicating that information in the local_contrib_review_outcome table for quicker lookups.
  • Group icons === awesome idea which I will be sure to work in.
  • Haha quite right it does support files, I've updated the doc now. I had given it a quick glance and misread the above else thinking anything with an underscore entered there..... just another reason I should read more carefully. Thanks for pointing that out.
  • Yes to both the capability suggestions, there had been talk about automating that some how and capabilities for it would be perfect.

Again thank you, I'll amend the rest of the doc shortly.

Cheers

--Sam Hemelryk 09:48, 12 January 2011 (UTC)

Initial comments from David

  • overall - sounds good! :-)
  • re links to AMOS - easy to implement now as AMOS already supports permalinks to its filter so it is easy to construct URL that leads the user directly to a page where a given component on a given branch is translated
  • what I miss in the document is a mention of some webservices support. We will either need webservice layer or maybe better just a script that generates plugins info into a big XML file. Such file will describe all the plugins and the location of their sources. This file will be then fetch regularly by Moodle sites' plugin_manager that will allow remote update/installation via the web. I am willing to work on this part once we branch MOODLE_20_STABLE, though there is no spec yet, sorry. The goal is to allow simple installations of extensions from the Moodle admin interface - similarly to what Wordpress does
  • that XML file would be used by AMOS, too, so that it knows what plugins and their branches it should track

--David Mudrak 10:57, 12 January 2011 (UTC)

Hi David

Thanks for looking it over and having a think about it. I've been working on the proposal again today and building it up more and more.

In reply to your points:

  • Awesome that AMOS has a predictable link structure, it certainly will be cool to integrate the two system.
  • There is now information about RSS feeds and web services that the new system will be able to process. The idea of having everything available from the admin interface of your Moodle installation is great. I'm not too sure presently what Martins plans for undertaking such work are, however he did ask for it to be provisioned where/if possible within this new system.
  • I would like to talk to you at some point after Martin is back to find out how we could best integrate this system with AMOS. It'd be great to see where we could take it and I'm sure you'd have the best idea about what is possible there. I'll get in touch with you after I've talked to Martin and organise a time to discuss it.

Cheers

--Sam Hemelryk 08:40, 13 January 2011 (UTC)


Why did you just strip local_ from the front of all the table names. I thought that all DB tables were meant to start with the frankenstyle plugin name?--Tim Hunt 10:26, 19 January 2011 (UTC)

Hi Tim, I removed local_ after finding the 28 character table name limit imposed by XMLDB. I choose to remove local_ rather than using cryptic names. --Sam Hemelryk 06:56, 21 January 2011 (UTC)
I know the 28 character table name limit is a pain, but we all have to live with it. I still don't see why you should be able to break the coding guidelines. I was specifying something yesterday, and eventually I was able to find table names that fitted and were sensible.
Also, while you claim your table names are non-crypic becuase they are long, actually, while trying to think of better names, I completely misunderstood what the collection thing was about. I thought it was for dealing with the situation where you have, for example, a block and an admin report that, while being separate plugins, rely on each other, so you want to offer them for download as a single package. However, when I read the detail of the document, I find that is wrong. So, collection is not only a long word, it is also ambiguous.
Anyway, here are my suggestion to get you within the limit:
review_criterion -> review_test. collection -> set (does not really solve the ambiguous naming, but at least is short. I think a significant part of your problem is that this concept overlaps very heavily with tags, but presumably you want tags to be crowd-sourced). Then I would cop out and just use an abbreviation for version, say ver or vers. That solves the rest of your problems.
  1234567890123456789012345678
  local_contrib_plugin
  local_contrib_vers
  local_contrib_vers_download
  local_contrib_category
  local_contrib_contributor
  local_contrib_set
  local_contrib_set_plugin
  local_contrib_software_vers
  local_contrib_supported_vers
  local_contrib_review
  local_contrib_review_test
  local_contrib_review_outcome
  local_contrib_awards
  local_contrib_plugin_awards
  1234567890123456789012345678
I think the best solution to the ambiguity of what some of the table names means is better table comments. At the moment, some of the comments only make sense if you have read the full spec, which will not be try for people who are just looking at the code.--Tim Hunt 08:43, 21 January 2011 (UTC)

Hi Tim, thanks for the feedback.

The 28 character table name limit is a bit of a pain, but I do see your point and happy to add local_ back into the mix and refactor the table names as suggested.

Your observations regarding collections are correct, it is very similar to tags however only those with the required capabilities will be able to manage/add to them. The name collections is perhaps a bit vague and as it will be way to long I think using set instead is a good idea.

I will also go through and revise the table comments :)

Cheers --Sam Hemelryk 01:41, 24 January 2011 (UTC)

typo?

  • 'Version control systems and downloads'

With this new system we will start collecting information about the version control system that the contributor is using. We gather this information for each version of a plugin that is created and are able to collect some or all of the following: 'the new of the vcs system'...

the new what of what system? --Joseph Rézeau 17:14, 2 February 2011 (UTC)