search

JujaLabs / itevents

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

Issue53 #149

Closed zavodnyuk closed 8 years ago

zavodnyuk commented 8 years ago

connect to #53

witjem commented 8 years ago

@zavodnyuk

  1. В класі MailReminderAboutEventService в методі sendEmails замість userWhoGoToNearestEvent краще використоти user, оскільки метод просто відправляє листи, він нічого не знає про юзерів
  2. Поправити конфлікти із мастером. Зараз у нас в проперті всі зміні називаються через крапку, тому cron.remindAboutEvent треба буде перейменувати на cron.remind.about.event і те саме із cron.start_sending
zavodnyuk commented 8 years ago

@vaa25 Thanks for your rewiev. @AndriyBaibak 2 hours

vaa25 commented 8 years ago
romach commented 8 years ago

@AndriyBaibak time spent: 30 minutes on code view

zavodnyuk commented 8 years ago

@AndriyBaibak 4 hours @alex-anakin refactored

vaa25 commented 8 years ago

@zavodnyuk

  1. All above.
  2. MyBatisEventService.getEventsByDate() used only in test. Dead code.
  3. MyBatisVisitLogService has unused imports
  4. VisitLogService has unused import
  5. Has changed changelogs
  6. etc...

Although this pr has my PASSED but merge it into maser now must not.

vaa25 commented 8 years ago

@AndriyBaibak 1 hour

vaa25 commented 8 years ago

@zavodnyuk try to compare your branch with master before pr please

zavodnyuk commented 8 years ago

@AndriyBaibak 4 hours

zavodnyuk commented 8 years ago

@AndriyBaibak 1h 30 min

romach commented 8 years ago

@zavodnyuk I get

org.springframework.beans.factory.BeanCreationException: Could not autowire field: private int org.itevents.service.sendmail.MailEventRemin
derService.daysTillEvent; nested exception is java.lang.IllegalArgumentException: Could not resolve placeholder 'days.till.event.to.remind' in string v
alue "${days.till.event.to.remind}"

                Caused by:                                        
                java.lang.IllegalArgumentException: Could not resolve placeholder 'days.till.event.to.remind' in string value "${days.till.event.to.rem
ind}"

while build

zavodnyuk commented 8 years ago

@romach ok, look to last commit. You need add property to local.proprties and it will be okey. Branch merged with master

romach commented 8 years ago

@AndriyBaibak 1 hour spent

zavodnyuk commented 8 years ago

@AndriyBaibak 30 min @romach All advices refactored