Code and project standards

[freesoul]

I created this issue in order to suggest some improvements for the reusability of the code. But I don't want to sound rude. Please contact me if you want to have exchanges about that :)

[maudetes]

Hello !
We're greatly interested for getting feedback on the reusability of udata. Don't hesitate to reopen and explain your pain points or suggested improvements.

For you information and seeing that it was targeted at the udata-piwik plugin originally, we plan to refactor the way we compute metrics, having some limitations inherent to Matomo/Piwik, thus redefining the scope of udata-piwik and udata-metrics plugins.

[freesoul]

Hello @maudetes , it is great to hear that, thank you !

While adapting the udata projects to the needs of the luxembourgish website, sometimes it was a bit hard to follow the data flux. I think these points would greatly help to understand how everything works:

• Typing in function arguments. This would let know directly which object or data type is expected as function argument. If it is a class, it allows the IDE even to let us knows about the methods.

• Avoiding wrappers. For instance, a function wrapper whose only function is to recover an environment variable and to return a class initialized with it. While using typing, we would have to pass the wrapper type instead of the real class type being used. I think it would be better to recover directly the environment variable. Another example are library wrappers (such as the paginator). I understand that sometimes we want to adapt the features to our code style, but losing the track of the original methods and variable names defined by the libraries are a huge drawback, as it it makes hard to use their documentation.

• Comments where it's hard to follow. While trying to understand the data flux (for instance, why our dataset views metrics is being reset to 0 in apparently random datasets across weeks), it would be extremely useful some short info in the methods that tells us where the data comes from and goes to.

• Code that stays in the projects but it is not useful anymore. Well this is self explanatory :D

• Centralize the constants. For instance, I had the task of changing the pagination number on the datasets. It was somewhat hard to find out which were the correct parameters to change. I had to change one in the Vue app, and another in the rest API. I think it is better to transfer the configuration from the API to the website, so it is in only one place. Regarding to the constants, it is a bit hard to understand which are the parameters being used, as they are defined in different ways: in a Settings class, read from the environment with an inline default, hardcoded in the base or inherited class, duplicated in the App, and so on.

• The plugin concept. I think that the main project should be done in a way in which it is completely independent from the plugins and not need to know anything from them, while of course allowing them. For instance, it is not clear if the metric "views" is a base feature or a plugin feature, as the definition of the variable and its use are in the main project, but what fills it is in the "plugin".

• Misleading/outdated documentation. This could lead to confusion easily, I remember I saw for instance that the udata.cfg file should be in the root of the projects, and in some other page that it should be inside the udata project itself, but then the config file route also could be read from the environment in some cases.

• Classes defined inside static methods (see class TempSearch). I think that it is better to use a separate container from the class handler to contain the data, which would prevent from the need of using inheritance to copy a class, which seems a bit hacky to me.

• Search results that come first from the html page then loaded from the vue component. I think that the speed earned from performing the rendering of the search page on server side (so it comes with the html), it is negligible, whereas loading directly the component with the search results would be more compact.

• Deprecation warnings. Maybe I did not see it and it would be my fault, but I was integrating the ES/Kafka plugin and a day later Kafka was removed :D

• Clear releases I could not use the last version of the search plugin as I could not find the new code or service responsible for consuming/inserting into ES the new data. We had to stay for the moment with Kafka. We also had a problem with this, not sure why: I can not find where the message is being published on the upload of a dataset (so we manually reindex the whole thing with a cron)

[freesoul]

I just noticed I probably should have opened this issue in etalab/udata-front, are you the same people or should I do that instead?

[maudetes]

Hello!
I haven't taken the time to answer because I wanted to answer in depth to the numerous points you mentioned.
But we will come back on this!

You opened this issue at the right place, we are the same people behind udata and udata-front (and most udata plugins). FYI, we plan on migrating udata back to the etalab organization. See datagouv/data.gouv.fr#887