search

mal-lang / mal-toolbox

Apache License 2.0
4 stars 2 forks source link

Remove unused function from neo4j.py #43

Closed nkakouros closed 1 month ago

nkakouros commented 5 months ago

I couldn't find get_model being used anywhere.

andrewbwm commented 5 months ago

Giuseppe requested this behaviour at some point and that's when the function was introduced too. I am all for a clean and simple code base, but I do not see much merit in removing this. You are right that we do need to maintain more things at the same time, but I wouldn't hack out existing functionality that some people might still be using,

nkakouros commented 5 months ago

we do need to maintain more things at the same time

And this results in unmaintained code like the one this PR removes.

If I understand what get_model does, it creates an instance model from a neo4j instance. I think this is clearly out of scope for the mal-toolbox, don't you think?

andrewbwm commented 5 months ago

we do need to maintain more things at the same time

And this results in unmaintained code like the one this PR removes.

If I understand what get_model does, it creates an instance model from a neo4j instance. I think this is clearly out of scope for the mal-toolbox, don't you think?

This was not something that was left over from some early iteration, it was something that Giuseppe specifically requested at the time. It was out of scope when it was introduced, the question if it's needed.

nkakouros commented 5 months ago

It's your call. My input is that the potential of seeing specication.py removed is great news. If you don't feel comfortable removing the securicad module, perhaps the relevant code from specification.py that is used only in securicad.py could go in there and specification.py is removed?