Forum OpenACS Q&A: Re: Chat package HEAD branch

Collapse
Posted by Malte Sussdorff on
Sorry for being a little bit harsh, but the changes you introduced were bad from a development point of view.

a) Do not make a package dependent of .LRN if it was not so before. There exist "if [apm_package_enabled_p dotlrn]" statements to make sure you can do that without harming non .LRN users

b) If you insert the community name into a room, do not make a query for the name in the __new function, but instead pass it as a parameter so people could actually decide for themselves in what context to name the room.

c) Please remember the naming conventions. It is chat_rooms_private__new (so __ in front of the action) not, chat_rooms__private_new. I did not change that.

d) Please provide language keys in English. I am aware that probably your application is written in spanish, but the main language of the toolkit is English, therefore having at least minimal language keys to know what is going on is helpful.

e) If you need to upgrade, upgrade the .info file as well with the version number so the upgrade can actually happen.

f) If you do not enable RSS Service as a user, do not execute the RSS creation statements when adding a room.

g) ad_schedule_proc is not being stored in the database, therefore if you call ad_schedule_proc and expect the rooms to stay open beyond a restart, that does not work.

h) You depend on chat-portlet in the message keys. Please do not do this. And if you want to do it, make it work the other way round so chat-portlet message keys work with the chat message keys.

j) If you need to set an id (like folder_id) user db_string, not db_1row to check how many there might be and then setting only one.

k) There are other ways to figure out a file storage folder of a community without the need to go over the title in acs-objects. Furthermore, why would you use just this one folder in the first place (chat.tcl)

This is only in three files, but i have given up now. It does not make any sense for me to try to get this to work outside .LRN and to fix in my opinion fundamental errors in development style. Please take a look at how other packages are written and try to make it work without .LRN.