search

JujaLabs / itevents

Resource to subscribe on it events
Apache License 2.0
7 stars 5 forks source link

Issue47_1 + issue47_2 #86

Closed romach closed 9 years ago

vaa25 commented 9 years ago
  1. maximumDate - максимальная дата? то есть через неделю по этому фильтру не придет ни одно письмо?
  2. offset не сохраняется в бд, он используется только при текущем просмотре по фильтру
  3. в GetFilteredEventsSqlBuilder дублирование кода. если вдруг изменится фильтр то нужно будет менять оба метода.
  4. boolean==true можно упростить
  5. buildMailFromXmlEvents копипаста. Мне кажется лишним получать файл через создание нового контекста.
  6. с TimeUtil на мой взгляд удачная идея. текущая дата используется и во многих других местах проекта. Я думаю неплохо было бы отрефакторить их на использование TimeUtils.
  7. getNowDate благозвучнее звучит как getCurrentDate, хотя на вкус и цвет...
  8. нет новых тестов в GetFilteredEventsSqlBuilderTest
  9. При сборке gradle стабильно падает: testMailBuild http://pastebin.com/BTMwHzNR
romach commented 9 years ago
  • maximumDate - максимальная дата? то есть через неделю по этому фильтру не придет ни одно письмо?
  • offset не сохраняется в бд, он используется только при текущем просмотре по фильтру

maximumDate задается в методе getFilteredEventsInDateRangeWithRating в зависимости от текущей даты;

  • buildMailFromXmlEvents копипаста. Мне кажется лишним получать файл через создание нового контекста.

Как по-другому его можно получить?

vaa25 commented 9 years ago

maximumDate задается в методе getFilteredEventsInDateRangeWithRating в зависимости от текущей даты;

Задается или берется из Filter?

Как по-другому его можно получить?

new File("...") например, или getClass().getResource("...")

romach commented 9 years ago

Задается или берется из Filter?

Задается

alex-anakin commented 9 years ago

See comments in code