Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

beta migration #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

beta migration #76

wants to merge 1 commit into from

Conversation

AIlkiv
Copy link

@AIlkiv AIlkiv commented Jul 18, 2017

Closes #7

@yukoff
Copy link
Contributor

yukoff commented Jul 18, 2017

Надалі пропоную користуватися "закриваючими словами GitHub"

@yukoff
Copy link
Contributor

yukoff commented Jul 18, 2017

Це ж по #7, я так розумію?

bb_auth_access aa
WHERE
ug.user_pending = 0
$users_in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AIlkiv Що за змінна? Бо

PHP Notice: Undefined variable: users_in in .../db/migrations/20170608215036_toloka_new.php on line 773

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @AIlkiv

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

злий копіпаст) Виправлю в новому PR, десь у суботу, неділю

Copy link
Contributor

@yukoff yukoff Jul 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AIlkiv Буває ;) Та в принципі особливо сенсу немає - я ці зміни адаптую до #46/#74 (doctrine migrations), хоча upstream рухаються до laravel, але через обмеження його ORM від Eloquent довелось відмовитись (хоча це не проблема взагалі). Тобто, якщо окрім одруків тощо нічого міняти не треба - можна й оновити, а якщо ще щось "пиляти" доведеться - то може краще через коментарі чи в gitter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AIlkiv З $users_in розібрався - просто прибрав, судячи з коду оригінальної ф-ції

*/
public function __construct()
{
$this->tpl = get_bbcode_tpl();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Шо? o_O

return (SQL_DEBUG && DBG_USER && !empty($_COOKIE['sql_log']));
}

function get_bbcode_tpl()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

фух... а нащо код torrentpier дублювати в файлі міграції?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Через те що вони юзають всюди глобалки і те що не можна без ініціалізації TP заінклудити їхні файли треба було тупо перенести деякі функції і класи. Враховуючи що це фактично разовий скрипт, тому не заморочувався над рознесенням по додаткових файлах

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зрозуміло, тільки от виклик глобальної ф-ції в конструкторі... Ну не знаю =) Чого не new BBCode(get_bbcode_tpl())? Ну, в принципі це більше з цікавості.

{
public function newTable()
{
$table = $this->table("bb_ads", ['id' => false, 'primary_key' => ["ad_id"], 'engine' => "InnoDB", 'encoding' => "utf8", 'collation' => "utf8_general_ci", 'comment' => ""]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А що це за таблиця? я щось її не бачу ані в старій, ані в новій базі

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Генератор міграції запускав на початку червня і тоді ще була дана таблиця. Прогляну diff install/mysql.sql і внесу потрібні зміни

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konfuciusu А хто в курсі, що за таблиця bb_ads? Бо не бачу я її ані в старій, ані в новій схемі.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Розібрався, вона мала дропнутися при апдейті на 2.2.0 https://github.com/hurtom/toloka/blob/master/install/upgrade/changes.txt#L9-L10

@ghost ghost added the needs review label Aug 2, 2017
yukoff added a commit to yukoff/toloka that referenced this pull request Aug 24, 2017
* Merge remaining queries, reorder and add references to issues/PRs
* Adjust datatype for topic_type
* Add validation for torrent check status
* Some source formatting etc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants