Стажировка, Haskell: разные уточнения

[antonkalinin-ml]

Опыт последнего ревью сервера показал, что требования можно реализовать настолько "в лоб", что пользы от проекта намного меньше, чем могло бы быть, но на ревью к этому трудно придраться. Вот же - в требованиях не написано, значит, необязательно. Заставлять переделывать весь проект не хочется, это во многом наш недочет, что требования можно понять неправильно и проделать массу работы впустую. Нужно ваше мнение по поводу доработок.

Срочное:

1. убрать упоминание только hindent. В #344 обсуждается, какой список форматтеров должен быть, но это надолго, а пока надо привести требования в актуальный вид.
2. написать, что все импорты должны быть либо qualified, либо с импорт листом. (Позже можно обсудить, настолько ли это надо).
3. добавить к серверу пункт про запрет произвольных либ. Явно написать, какие можно: warp, wai, cryptonite, для конфига, все из haskell platform, библиотека для базы, но без автоматических миграций. У нас есть список, но он лежит в разделе про бота.
4. добавить в источники ссылку на статью Олега, так как в чате есть вопросы про Handle Pattern. Существующие источники предлагают не париться и писать все хендлы прямо в IO, это немного не то, что мы хотим.
5. про бот написать, что проект должен быть один, а не два.
6. В бот и сервер добавить подсказку "перед тем, как делать, прочитай требования в задании 5".

Менее срочное, но важное:

7. сделать требования для API более детальными, вплоть до названий эндпойнтов, методов и параметров в урле. Формат request body не будем указывать, с этим все справляются. Это нужно, чтобы люди попробовали разные способы передачи параметров (в пути, в квери), а не передавали все через боди в один эндпойнт, включая GET-запросы. Однажды в будущем, может быть, у нас будет постман-коллекция для тестов. У нас упоминается, что нужно использовать принципы REST, но это сложная, полумифическая штука, которая далеко не везде нужна и нарушается где только можно (включая нас), не стоит заставлять людей тратить на это время, а потом переделывать. Лучше дадим готовый образец API, как надо, и этого во многом будет достаточно (на моем последнем проекте - более чем). Недостаток: когда API жестко задано, то все проекты более похожи и есть ощущение, что чужой проект легче выдать за свой (хотя для этого и сейчас нет никаких препятствий, кроме бдительного ревью и собеседований).
8. картинка должна возвращаться в бинарном виде, с заголовком Content-Type: image/png, либо попросим явно передавать контент-тайп в запросе, хранить его в базе и возвращать. Новость и черновик должны включать в себя урлы картинок. Для создания картинки можно послать ее содержимое в base64 в JSON, чтобы не заморачиваться с multipart/form-data.
9. выбрать один простой способ авторизации: basic auth (или, может, bearer), никаких куков и прочей экзотики :)
10. что должно быть можно задать в конфиге:
    • подключение к базе: имя базы, юзер, пароль, хост, порт. Чтобы ревьюер мог создать базу, как ему удобно, а не возиться со скриптами в проекте.
    • минимальный уровень логирования - чтобы посмотреть на дебаг-сообщения, не правя код.
11. запрос на редактирование сущности должен позволять редактировать любое количество любых полей сущности - любое одно, любые два, все сразу. (Видел, когда реализован только вариант "все сразу" ).
12. использовать cryptonite - это не требование, а подсказка. Можно и другими способами, например, через постгрес.
13. должна быть возможна фильтрация новостей одновременно по нескольким критериям и одновременно с сортировкой. Фильтры и сортировку передавать в квери.
14. вложенные сущности предлагаю слать в ответах везде, а не только в новостях. Это должно побудить переиспользовать код.
15. Для категории описать формат сущности в API: массив с иерархией категорий, от корня к нужной категории, например [{"category_id": 1, "name": "Programming"}, {"category_id": 5, "name": "Haskell"}]. В базе категории представляются совсем не так, потребуется написать кастомную функцию преобразования.
16. в требования к ревью дописать, что код должен переиспользоваться, а не копипаститься. Должна быть возможность при помощи единственной правки изменить формат вывода (перечень полей и имена) любой сущности сразу везде.
17. расписать, что такое юнит-тесты (они не должны задействовать веб и базу), и хорошо бы даже привести обязательные тесткейсы. е2е-тесты не годятся - чтобы их написать, не нужны никакие Handle Pattern/ReaderT, а для юнит-тестов нужны. Только я затрудняюсь с кейсами, основная функциональность это вынуть-положить в базу. Можно добавить следующие требования и попросить, чтобы они были проверены в тестах:
    • все теги должны иметь уникальные имена. Это условие надо проверять при создании и редактировании тега.
    • все категории внутри одного родителя должны иметь уникальные имена. Это нужно проверять при создании и редактировании.
    • при редактировании категории не должно образовываться циклов, т.е. категория не может быть своим собственным родителем или потомком. Проверять при редактировании.
    • пару кейсов на state-based testing, когда надо удостовериться, что в базе, допустим, удаляется именно та категория, которая требуется, но ничего другого. В тестах подменяются вызовы к базе, делается моковая "база", все попытки удаления записываются в IORef и потом проверяются.
18. в задание 5 добавить пункт "перед отправкой проекта на ревью пробегись по требованиям и проверь, что все сделано". Это сэкономит ревьюеру некоторое время на перепалках по поводу форматтеров, хлинта и разворачивания базы :)

Надо уточнить:

19. какие условия для удаления сущности? Проще всего не удалять, если сущность используется, но на практике это не очень полезно. Но мне не хочется усложнять требования, задание и без того объемное.

[olgaklimenko]

1. Откомментила в ишью по форматтерам
2. + На проектах обычно придерживаемся
3. + Надо найти максимально приближенный к нашим требованиям по использованию библиотек проект и оттуда взять список. Если случайно дадим неполный список начнётся изобретение велосипедов (такое уже встречалось).
4. + Статья Олега обычно очень помогает, 100% надо добавлять
5. + Можно сюда вот дописать это требование. Специально уточняла про запуск 1 бота из двух через указание в конфиге, но видимо надо уточнить ещё.
6. + И перед тем как звать на ревью, перед тем как коммитить новый код после ревью - тоже лучше смотреть в задание 5.
7. - Тут не очень хочется ограничивать творчество стажёра, хочется чтобы он сам придумал схему АПИ, точно сам разобрался где какой метод использовать и тд. И на ревью (мне) не так сложно проверить и написать ишью, чтобы стажер поразбирался ещё раз. Может достаочно будет добавить хороший ресурс в задание. Возможно я просто не встречала проекты стажеров с большими проблемами с АПИ, если есть примеры можно кратенько рассказать что там было.
8. +
9. +
10. +
11. +- Не знаю, может не стоит усложнять. Пусть хотя бы сделают обновление всего сразу.
12. + Надо дописать, что подсказка. Если будем список библиотек делать из п.3, то надо будет это учесть.
13. +
14. +
15. +- Тут тоже не хочется усложнять. С одной стороны это разнообразить задание, с другой добавит время на его выполнение и ревью.
16. +
17. + Новые требования понравились, как раз чтобы разнообразить тестирование.
18. +
19. + Можем сделать тут наиболее мягкое требование, или написать варианты и пусть сами выбирают по желанию (а варианты как раз наведут их на то, чтобы задуматься немного и выбрать).

[stanislav-az]

Может все требования к заданиям перенести в git? Чтобы можно было легко понимать какие требования изменились.

[stanislav-az]

Я согласен со всеми изменениями. Вот кроме этой части в тестах

все попытки удаления записываются в IORef и потом проверяются

Думаю лучше требовать сделать чистые юнит тесты, с простым StateT, чтобы всё было без IO. Тогда не возможно оставить что-то из реальной имплементации которая и работает в IO.

Насчёт форматтера отдельно - думаю для хаскеля нет готового хорошего форматтера, но ормолу хотя бы работает, поэтому логично требовать его.

[antonkalinin-ml]

Может все требования к заданиям перенести в git? Чтобы можно было легко понимать какие требования изменились.

Это надо уговаривать менеджеров по стажировке. Этот вариант уже вроде рассматривался, когда уходили из риззомы, и решение было не в пользу гитхаба :(. Можно перенести только задания, они их не правят, но программа обучения на 90% состоит из заданий, и если их переносить, то в коде останется только введение и FAQ :).

[antonkalinin-ml]

Возможно я просто не встречала проекты стажеров с большими проблемами с АПИ, если есть примеры можно кратенько рассказать что там было.

В одной работе видел:

• GET-запросы с боди. Ни в одном запросе параметры не передавались в URL (в пути или в квери), все через боди.
• PUT-запрос принимает в request body данные не того формата, в котором возвращает соответствующий GET. Это легко исправить, можно использовать PATCH.
• раздельные эндпойнты для разных фильтров новостей, например /news/by_tag, /news/by_category, что исключает их использование вместе. Если укажем API точнее, таких ошибок не будет.

Когда я делал сервер, сильно заморачивался, так как не знал, насколько хорошо надо реализовать ресты. Например, при создании тега я мог вернуть 201 Created, если создан новый тег (+ какой-то заголовок с URI этого тега), либо 200 ОК, если тег с таким именем уже есть, и дополнительный заголовок с урлом найденного тега. Все это было не нужно.

[olgaklimenko]

раздельные эндпойнты для разных фильтров новостей, например /news/by_tag, /news/by_category, что исключает их использование вместе. Если укажем API точнее, таких ошибок не будет. Это нужно в задании уточнить.

Два оставшихся примера наверно можно покрыть ресурсами. Но всегда будут появляться какие-то индивидуальные ошибки, потому что не прочитали или ресурс не покрыл какой-то особый кейс из головы стажёра.

Если мы предоставим готовую схему АПИ, такие ошибки исчезнут, но никто и не будет пробовать разработать схему самостоятельно. Надо исходить из того насколько джуну-1 надо уметь это делать. Думаю, первому джуну никто не будет поручать такие задачи (хотя тут тоже от проекта зависит).

[antonkalinin-ml]

Согласен, нехорошо отбирать разработку схемы. Тогда можно написать перечень требований, что там должно быть: должна быть передача параметров в урле и пр.