wvulibraries / mfcs

Archival and Preservation Digital Asset Management System.
Other
8 stars 4 forks source link

Exporting update #244

Closed MichaelRBond closed 7 years ago

MichaelRBond commented 7 years ago

Exporting updates to support auto loading into an external system.

It is possible that some external systems may need additional information in the control file. However, it should be trivial to add additional control files. Some code will need to be changed. It may be needed to setup an map of replacements for control files. This situation should be addressed IF it ever becomes a concern.

MichaelRBond commented 7 years ago

yaml file extension is fine. i is more descriptive than yml, and all mdern operating systems can handle more than 3 character file extensions.

MichaelRBond commented 7 years ago

adding another function, in place of the regular expressions is ridiculous. its adding another function call, where it is absolutely not needed, less concise, more difficult to read (because you have to bounce around), and harder to maintain in the long run. I will not make that change.

If you would like that change, go ahead and make it. but the first time i have to maintain that code, i wiil revert it your commit, and start from there.

MichaelRBond commented 7 years ago

I'll add documentation to the wiki when the process is done.

ddavisgraphics commented 7 years ago

@MichaelRBond Please show me how this

function determine_add($field,$type) {
        if (strtolower($type) == "label") {
            return $field['label'];
        }
        else if (strtolower($type) == "name") {
            return $field['name'];
        }
        else if (strtolower($type) == "class") {
            return $field['class'];
        }
        else if (strtolower($type) == "id") {
            return $field['id'];
        }
        return "";
    }

is more readable than

 public static function determine_add($field,$type) {
        switch (strtolower($type)){
            case "label":
                return $field['label']; 
            case "name":
                return $field['name']; 
            case "class":
                return $field['class']; 
            case "id":
                return $field['id']; 
            default:
                return "";
        }
    }

I don't know what your talking about adding more functions for regular expressions.

MichaelRBond commented 7 years ago

I was taking a guess, because you didn't reply with a line comment. Maybe I guessed wrong.

Please reply with line comments

Get Outlook for iOShttps://aka.ms/o0ukef


From: David J. Davis notifications@github.com Sent: Monday, November 14, 2016 12:35:16 PM To: wvulibraries/mfcs Cc: Michael Bond; Mention Subject: Re: [wvulibraries/mfcs] Exporting update (#244)

@MichaelRBondhttps://github.com/MichaelRBond That is crazy to assume that

function determine_add($field,$type) { if (strtolower($type) == "label") { return $field['label']; } else if (strtolower($type) == "name") { return $field['name']; } else if (strtolower($type) == "class") { return $field['class']; } else if (strtolower($type) == "id") { return $field['id']; } return ""; }

is more readable than

public static function determine_add($field,$type) { switch (strtolower($type)){ case "label": return $field['label']; case "name": return $field['name']; case "class": return $field['class']; case "id": return $field['id']; default: return ""; } }

I don't know what your talking about adding more functions for regular expressions.

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/wvulibraries/mfcs/pull/244#issuecomment-260403733, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABadrIkLSPUDQvzTEiVq4W1CyXtcz1ofks5q-JvUgaJpZM4KwrMR.

ddavisgraphics commented 7 years ago

It is line 39 on exporting, but I guess it isn't a changed part of the file in your main PR so that can be adjusted later. Still the main issues are with documentation of methods and documentation on how it is supposed to work and what it accomplishes.