{"action":"opened","issue":{"url":"https://api.github.com/repos/YunoHost-Apps/piwigo_ynh/issues/11","repository_url":"https://api.github.com/repos/YunoHost-Apps/piwigo_ynh","labels_url":"https://api.github.com/repos/YunoHost-Apps/piwigo_ynh/issues/11/labels{/name}","comments_url":"https://api.github.com/repos/YunoHost-Apps/piwigo_ynh/issues/11/comments","events_url":"https://api.github.com/repos/YunoHost-Apps/piwigo_ynh/issues/11/events","html_url":"https://github.com/YunoHost-Apps/piwigo_ynh/issues/11","id":259877765,"number":11,"title":"Corrections and enhancements","user":{"login":"maniackcrudelis","id":8302225,"avatar_url":"https://avatars2.githubusercontent.com/u/8302225?v=4","gravatar_id":"","url":"https://api.github.com/users/maniackcrudelis","html_url":"https://github.com/maniackcrudelis","followers_url":"https://api.github.com/users/maniackcrudelis/followers","following_url":"https://api.github.com/users/maniackcrudelis/following{/other_user}","gists_url":"https://api.github.com/users/maniackcrudelis/gists{/gist_id}","starred_url":"https://api.github.com/users/maniackcrudelis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/maniackcrudelis/subscriptions","organizations_url":"https://api.github.com/users/maniackcrudelis/orgs","repos_url":"https://api.github.com/users/maniackcrudelis/repos","events_url":"https://api.github.com/users/maniackcrudelis/events{/privacy}","received_events_url":"https://api.github.com/users/maniackcrudelis/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-09-22T17:01:46Z","updated_at":"2017-09-22T17:01:46Z","closed_at":null,"author_association":"NONE","body":"Because is about to become an official app, there are some things we have to discuss about some parts of the code.\r\nLet's go :)\r\n\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/conf/nginx.conf#L3\r\nAre you sure it works without a equivalent setting in your php config ? Did you already test it ?\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/backup#L25\r\nCan you use the final_path stored in the config instead of set it here. You store this value in your instal script.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/change_url#L64\r\nDidn't we change this line somewhere else to have a more simple version of this sed ? Or I just tried and didn't make it ?\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/install#L2\r\nAre you sure you really need that ?\r\nYou don't use any glob inside a double quotes. And it works fine without double quotes. If you need that, just explain a little more what is it.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/install#L103\r\nCan you use instead the Picture directory of yunohost.multimedia ? In order to not duplicate this media.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/install#L121\r\nIt's already done by the helper just above.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/install#L123-L173\r\nCan you add here more \"big\" comment to structure the code.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/install#L127-L129\r\nWhy a sleep here ?\r\nAnd these 2 reload are not needed, the respective helpers reload their service.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/remove#L4\r\nI think it's really a bad idea, if this script fail, the app will be in a non-installed / non-removed status. So, you can't remove it, and you can't reinstall it.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/remove#L39\r\nAre you sure you really want to remove all the pictures and photos of the users ?\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/remove#L54-L56\r\nThe previous helpers reload their service.\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/upgrade#L27-L69\r\nPlease more comments and blank lines ;)\r\n- https://github.com/YunoHost-Apps/piwigo_ynh/blob/master/scripts/upgrade#L73\r\nIs it lost ?\r\n- And same things than the install script for the upgrade script."}}
{"actor":{"display_login":"maniackcrudelis"}}