Hi Tony,
Bringing in lists into list_options with contrib forms like this has not done before (note we didn't accept the cardiology forms because of making a bunch of lists in list_options) and that mechanism needs to be reviewed before getting into the codebase. I quickly perused the code and my initial concerns are the potential for list_options conflicts in the futur. To be safe, I've reverted the commit and suggest instead placing it up for a code review. Sorry to do this, but best to get input before going ahead and placing it into the main codebase.
Also, regarding the edit issues. I used to think the same way about not being able to edit. But then I realized these messages get emailed to several hundred people, which an edit won't fix. Perhaps I'm just rationalizing, though :)
-brady OpenEMR
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Been looking into this a little more. Looking through the xmlformgen script, it appears that this addition to the list_options table is standard usage in there, so glad that Tony's forms have highlighted this. One solution is to p pre-pend all list_option ids (just the id of the list, not each entry) that are for forms with something (such as fxml_) to ensure don't clash with any of the core lists in list_options. However, this still doesn't avoid clashing with other forms; a nice solution that would solve this issue would be to instead create a form specific lists table (form_<form-name>_lists) that holds these lists, which wouldn't be too tough to add to xmlformgen script and could even allow modification of them via some "connector" code in Administration->Lists.
I don't really appreciate you reverting my commit. But I understand why you thought it should be, no worries.
Some of these forms were created initially using XMLFormGen and modified after the fact to do things that they would not otherwise do.
Contrib forms are just that contributions, they should
1) not be subject to massive review
2) be caveat Emptor when used in the field.
The whole point is not to have high barriers for things in contrib. No one should ever use anything in contribs without looking at it carefully and testing it in a safe environment.
Frankly, I think the contribs directory is full of broken crap right now. I really think that stuff should NOT be in the repo at all, but just made available in a separate REPO called openemr-contribs. Or just as drop in ZIP/TAR files on the web. I tried to do that on the wiki pages, but it went nowhere.
If people want the GYN forms I'll push the branch to my github, feel free to pull them down. Not going to spend any time, myself, making changes to them. Too many real jobs to do.
Tony
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Disagree with your view on the contrib directory. I agree there is garbage in there, but that doesn't mean it's ok to add more. Most logical thing to do is for somebody to spend time testing the stuff and keeping what works while removing/fixing the rest. Please understand that I am not saying your forms are garbage, but I do think they merit review for several reasons: 1.Two of the forms already exist in xml form in xmlformgen directory 2.These appear to be the only forms in the contrib directory that are hard-coded to directly add lists to list_options)
If you can spare the time from your "real jobs" to send me an email, then please send me (or post to github) your original files that you feed into xmlformgen, since this format is the ideal way to contribute these forms. I'll spend time on this myself converting them and comparing them to your current forms(if they are the same, then it will be a no brainer to contribute the xml file only); and the ones that differ significantly, then potentially some thought will be required.
And regarding the xmlformgen script, after the release, plan to spend some time improving this mechanism (ie. not dumping lists into list_options in this way). This is what makes uploading the xml files best, because any user that creates the forms will build from the most uptodate xmlformgen script (ie. a way to avoid forms in contrib from becoming outdated).
I will be cleaning up the forms in contrib directory…will take atleast a week for testing and giving back the feedback regarding the forms. Will tell which form works and which doesnt…
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Although a bit off topic, regarding xmlformgen, is there anything stopping OpenEMR from just using the raw xml files (like the CCR/CCD stuff does) rather than need to build all the files (ie. make them on the fly)? Julia, is this a possibility?
All the code I have has was included. XMLFormgen is great for what it does, but it's just as easy to write the code using PHP, the XML stuff does not support all field types.
Many times I have to create lists that are needed for forms, just as many times the lists need to be used by more than one form.
I think a new thread is needed to discuss form creation and sharing as a general topic.
By "Real Jobs" I mean things I get paid for. I don't have an employer to fund this as a hobby. Ask Rod how that works and try not to be condescending toward those of us who are trying to make a living as OpenEMR developers, please.
-Tony
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Tony I've been posting things up for review where in the past I'd just commit them, even though I didn't think anyone would care. My last two commits were like that. It's sometimes surprising the comments and suggestions that come up.
Thanks for putting that on github.
Rod
www.sunsetsystems.com
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
No other professional vendors/developers have openly refused to entertain the review process (and a review hasn't even yet been completed yet… :) ) as you are doing, so I was teasing you on that point. Sorry if that offended you; it was not meant to.
Dudes, I have posted everything I've ever committed to the review process except this set of contributed forms. So back way off.
I, personally, just don't think the contribs stuff warrants a big review process, but It doesn;t bother me if that's what you want to do. It's fine. The code has been shared as a branch on my github review away.
-Tony
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Think there's a bit of a misunderstanding here. I was referring to your comment above: "If people want the GYN forms I'll push the branch to my github, feel free to pull them down. Not going to spend any time, myself, making changes to them. Too many real jobs to do."
I thought you were refusing to make any code changes even before the review, which I haven't seen a vendor/developer do before. At this point, though, I realize this is not what you meant, and my responses are simply a misunderstanding. Please just disregard my comments above.
I just placed a review on github.
sincerely,
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I am reaching out to find information about if any OBGYN users have standard forms developed for their practice
I have a set of GYN forms, not much on the OB side. Be happy to post them in the contribs dir ( been meaning to, in fact)
affirm_sheet/
dub_soap/
gyn_ros/
pelvic_soap/
urinary_soap/
vaginitis_soap/
Tony
www.mi-squared.com / @tonymi2
oemr.org / @OEMR_org
That'd be quite helpful. So far, we've been making our own forms for various kinds of exams (hematological etc), using the LBF method.
I'll post them to the contribs dir in 411-dev later today. They should work fine in rel-410 too.
-Tony
I have push these form to the contrib dirs in dev-411
3c76a67 Contributed GYM Forms, some of the table.sql files include inserts into list_options for supporting selections
Tony
www.mi-squared.com / @tonymi2
oemr.org / @OEMR_org
I NEVER see the typo's until after I send the post, I HATE this forums lack of post edit capability… grrr
-Tony
Hi Tony,
Bringing in lists into list_options with contrib forms like this has not done before (note we didn't accept the cardiology forms because of making a bunch of lists in list_options) and that mechanism needs to be reviewed before getting into the codebase. I quickly perused the code and my initial concerns are the potential for list_options conflicts in the futur. To be safe, I've reverted the commit and suggest instead placing it up for a code review. Sorry to do this, but best to get input before going ahead and placing it into the main codebase.
Also, regarding the edit issues. I used to think the same way about not being able to edit. But then I realized these messages get emailed to several hundred people, which an edit won't fix. Perhaps I'm just rationalizing, though :)
-brady
OpenEMR
Hi,
Been looking into this a little more. Looking through the xmlformgen script, it appears that this addition to the list_options table is standard usage in there, so glad that Tony's forms have highlighted this. One solution is to p pre-pend all list_option ids (just the id of the list, not each entry) that are for forms with something (such as fxml_) to ensure don't clash with any of the core lists in list_options. However, this still doesn't avoid clashing with other forms; a nice solution that would solve this issue would be to instead create a form specific lists table (form_<form-name>_lists) that holds these lists, which wouldn't be too tough to add to xmlformgen script and could even allow modification of them via some "connector" code in Administration->Lists.
Tony, did you create these forms via xmlformgen?
-brady
OpenEMR
Brady,
I don't really appreciate you reverting my commit. But I understand why you thought it should be, no worries.
Some of these forms were created initially using XMLFormGen and modified after the fact to do things that they would not otherwise do.
Contrib forms are just that contributions, they should
1) not be subject to massive review
2) be caveat Emptor when used in the field.
The whole point is not to have high barriers for things in contrib. No one should ever use anything in contribs without looking at it carefully and testing it in a safe environment.
Frankly, I think the contribs directory is full of broken crap right now. I really think that stuff should NOT be in the repo at all, but just made available in a separate REPO called openemr-contribs. Or just as drop in ZIP/TAR files on the web. I tried to do that on the wiki pages, but it went nowhere.
If people want the GYN forms I'll push the branch to my github, feel free to pull them down. Not going to spend any time, myself, making changes to them. Too many real jobs to do.
Tony
Hi Tony,
Disagree with your view on the contrib directory. I agree there is garbage in there, but that doesn't mean it's ok to add more. Most logical thing to do is for somebody to spend time testing the stuff and keeping what works while removing/fixing the rest. Please understand that I am not saying your forms are garbage, but I do think they merit review for several reasons: 1.Two of the forms already exist in xml form in xmlformgen directory 2.These appear to be the only forms in the contrib directory that are hard-coded to directly add lists to list_options)
If you can spare the time from your "real jobs" to send me an email, then please send me (or post to github) your original files that you feed into xmlformgen, since this format is the ideal way to contribute these forms. I'll spend time on this myself converting them and comparing them to your current forms(if they are the same, then it will be a no brainer to contribute the xml file only); and the ones that differ significantly, then potentially some thought will be required.
And regarding the xmlformgen script, after the release, plan to spend some time improving this mechanism (ie. not dumping lists into list_options in this way). This is what makes uploading the xml files best, because any user that creates the forms will build from the most uptodate xmlformgen script (ie. a way to avoid forms in contrib from becoming outdated).
-brady
OpenEMR
I will be cleaning up the forms in contrib directory…will take atleast a week for testing and giving back the feedback regarding the forms. Will tell which form works and which doesnt…
Hi (especially Julia),
Although a bit off topic, regarding xmlformgen, is there anything stopping OpenEMR from just using the raw xml files (like the CCR/CCD stuff does) rather than need to build all the files (ie. make them on the fly)? Julia, is this a possibility?
Here's a quick section on the Active Projects page for planned xmlformgen improvements:
http://www.open-emr.org/wiki/index.php/Active_Projects#XMLFormGen_script_upgrade
-brady
OpenEMR
Hi Arnab,
Sounds great. Will be nice to get contrib directory cleaned up a bit.
-brady
The reverted contribution is available at: https://github.com/tmccormi/openemr/tree/gyn_contribs for anyone that wants them.
All the code I have has was included. XMLFormgen is great for what it does, but it's just as easy to write the code using PHP, the XML stuff does not support all field types.
Many times I have to create lists that are needed for forms, just as many times the lists need to be used by more than one form.
I think a new thread is needed to discuss form creation and sharing as a general topic.
By "Real Jobs" I mean things I get paid for. I don't have an employer to fund this as a hobby. Ask Rod how that works and try not to be condescending toward those of us who are trying to make a living as OpenEMR developers, please.
-Tony
Tony I've been posting things up for review where in the past I'd just commit them, even though I didn't think anyone would care. My last two commits were like that. It's sometimes surprising the comments and suggestions that come up.
Thanks for putting that on github.
Rod
www.sunsetsystems.com
Hi Tony,
No other professional vendors/developers have openly refused to entertain the review process (and a review hasn't even yet been completed yet… :) ) as you are doing, so I was teasing you on that point. Sorry if that offended you; it was not meant to.
I'll place a review on github.
thanks,
-brady
OpenEMR
Dudes, I have posted everything I've ever committed to the review process except this set of contributed forms. So back way off.
I, personally, just don't think the contribs stuff warrants a big review process, but It doesn;t bother me if that's what you want to do. It's fine. The code has been shared as a branch on my github review away.
-Tony
Sorry Tony,
Think there's a bit of a misunderstanding here. I was referring to your comment above: "If people want the GYN forms I'll push the branch to my github, feel free to pull them down. Not going to spend any time, myself, making changes to them. Too many real jobs to do."
I thought you were refusing to make any code changes even before the review, which I haven't seen a vendor/developer do before. At this point, though, I realize this is not what you meant, and my responses are simply a misunderstanding. Please just disregard my comments above.
I just placed a review on github.
sincerely,
-brady