Project

General

Profile

Actions
Copy link

Bug #2934

closed

possible None.get in EventLogServiceImpl

Added by Vincent MEMBRÉ about 12 years ago. Updated about 12 years ago.

Status:
Released
Priority:
2
Assignee:
Vincent MEMBRÉ
Category:
Web - Maintenance
Target version:
2.4.0~rc1
Pull Request:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
Name check:
Fix check:
Regression:

Description

in method getListOfModificationEvents, there is an Option.get

we should at least pu a default value with a getOrElse

What do you think of 0 as default value ?

repository.getEventLogByCriteria(Some("eventtype in (" eventList ") and id > " +lastSuccess.id.get ), None, Some("id DESC") )

Actions
Copy link
 #1

Updated by Arthur ANGLADE about 12 years ago

  • Status changed from New to Discussion
Actions
Copy link
 #2

Updated by Arthur ANGLADE about 12 years ago

  • Assignee set to Nicolas PERRON

It seems like Nicolas is able to answer this question

Actions
Copy link
 #3

Updated by Arthur ANGLADE about 12 years ago

  • Assignee changed from Nicolas PERRON to Nicolas CHARLES

wrong nicolas

Actions
Copy link
 #4

Updated by Nicolas CHARLES about 12 years ago

  • Status changed from Discussion to 8
  • Assignee changed from Nicolas CHARLES to Vincent MEMBRÉ

I think that 0 is a sane default value since the minimum id is 1 (based on the postgresql SQL).
You can do the correction Vincent

Actions
Copy link
 #5

Updated by Vincent MEMBRÉ about 12 years ago

  • Status changed from 8 to Pending technical review
  • % Done changed from 0 to 100
Actions
Copy link
 #6

Updated by Nicolas CHARLES about 12 years ago

  • Status changed from Pending technical review to Released

This is good. But Vincent, can you explain why two commits ? Is it a merge that didn't tell its name, or did you really commit on both 2.4 and master ????

Actions
Copy link
 #7

Updated by Vincent MEMBRÉ about 12 years ago

That's a merge !! there was only one commit in that merge, so it was considered as a commit, not as a merge

Actions
Copy link

Also available in: Atom PDF