search

mesnitu / EasyPopulate4BookX

Data import and export module for Zencart 1.3.x and 1.5.x
GNU General Public License v2.0
1 stars 0 forks source link

Unnecessary inclusion of EP4 files #9

Closed mc12345678 closed 8 years ago

mc12345678 commented 8 years ago

For what it's worth, the goal of this "joined" process was that the features added to support the BookX product type would be offered on top of the EP4 code without needing to include the EP4 code.

My suggestion would be (upon agreement/completion of managing the merge of the code), would be to completely remove the shared EP4 files and continue to maintain this plugin to EP4 much like one controls a plugin to ZC... If every ZC plugin included the entire ZC code, then the overall process would be ridiculous. Additionally, with each version of EP4, then the same code would need to be updated here although no change to the EP4 code is expected to occur that would affect this code...

In other words, once the two/three issues requested to address have been addressed there would be unnecessary updating of this plugin to incorporate the EP4 code changes. It will unnecessarily become a mess. But, I leave it to you to determine how you wish to proceed. It is your addition to EP4.

mesnitu commented 8 years ago

Yep, I was thinking that too. I wrote that on the the updated readme file. And here are my thoughts: First: your work on that class open enormous possibilities ( I think ), for who ever wants to make changes in code, style or EP4 behavior. And I thank you for that. I think I didn't even scratch the surface of those possibilities. I don't even know if I will ever. I'm guessing that this notifiers could be use upon most situations like this bookx stuff. So another fields, another mood. Secondly: It's quite certain that EP4 is going to evolve faster, and I've done this because I needed it , but coding it's not my professional live. It will be hard to keep track and merging and all that with all new versions. Third: As a plugin, not only this one, but whatever comes along , EP4 should not be responsible for some bad code.

EP4 could have a kind of list of external plugins ( something like that), that ofcourse could be more or less tested, but in no way responsible for it ( if I'm explaining wright )

Of course, that for me it would be much safe / secure, to have real coders looking into it, specially with zencart knowlegde, in terms of security, etc.... but that's live.

So I'll leave for the moment , because this is still real beta beta, ie: just notice that on v151 most of the export querys don't even work. But when it's done, and I can finally start working on the shop ( that's the all point ), I could remove the EP4 related files. But it's not my priority right now.

mc12345678 commented 8 years ago

Recommend removing the EP4 related files from this branch at this time unless there remains some reason that they are specifically different from the EP4 files managed elsewhere.

It would appear that this code now supplements that code and maintaining those files included will become a nightmare and likely cause confusion for those that use your added code.

mesnitu commented 8 years ago

Im on mobile, but I guess you're saying make a different repository. I will do that, but still didn:t had the time

mc12345678 commented 8 years ago

No, not saying to create a different repository. Saying simply delete all of the files that are in the eP4 plugin from this repo. This would leave just the bookx responsible items.

mesnitu commented 8 years ago

Now only the ep4bookx plugin exists, so this can be close.