Uploaded image for project: 'Plugins'
  1. Plugins
  2. CONTRIB-4338 META: Portfolio plugin for Evernote
  3. CONTRIB-4630

Minor improvements raised during last peer review

    XMLWordPrintable

    Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5.2
    • Fix Version/s: None
    • Component/s: Portfolio: Evernote
    • Labels:
      None
    • Affected Branches:
      MOODLE_25_STABLE

      Description

      1. Your private variables have no reason to be private, they should be protected.
      2. $this->settingprefix could be a class constant.
      3. In export_config_form(), you should use html_writer::link() instead of using raw HTML.
      4. In get_export_summary(), you should localise the keys of the array. If I understand correctly they are used to be displayed to the user.
      5. In send_package(), you're getting the content, and the mimetype from 3 different places, this could be simplified and set in variables at the beginning of the foreach.
      6. In send_package(), can you explain why the logic is different when the filepath is '/' or not? It is expected that the way portfolio system works?
      7. It seems that getenml() could be a static method, no reason to have that bound to an instance, and its called methods too.
      8. In reform_style_attribute(), when you remove the attribute, you remove the lowecased one, shouldn't that be nodeName?

        Attachments

          Activity

            People

            Assignee:
            thatsvishalhere Vishal Raheja
            Reporter:
            fred Frédéric Massart
            Participants:
            Component watchers:
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: