Skip to content

Conversation

@weralabaj
Copy link
Contributor

Allow for nullable dates (SentTime header) for messages list and failed messages.

@weralabaj
Copy link
Contributor Author

@johnsimons This is internal change, I think we don't need include it in an announcement, given that we'll provide this information in SP and SI (next version) releases.
SC changes itself don't impact customers directly, only views will be different in SP and SI.

@weralabaj
Copy link
Contributor Author

That can be released in 1.6. @johnsimons Can I merge it?

@johnsimons
Copy link
Member

To me this looks good, @WolfByte thoughts ?

@mikeminutillo
Copy link
Member

It looks good to me. My only concern is that any records in the DB which
already have DateTime.MinValue will carry on doing so.

On Tue, 4 Aug 2015 at 16:16 John Simons notifications@github.com wrote:

To me this looks good, @WolfByte https://github.com/wolfbyte thoughts ?


Reply to this email directly or view it on GitHub
#491 (comment)
.

@johnsimons
Copy link
Member

@weralabaj 👍 to merge

@johnsimons johnsimons added this to the 1.6.0 milestone Aug 4, 2015
weralabaj pushed a commit that referenced this pull request Aug 4, 2015
Return nulls when TimeSent is missing
@weralabaj weralabaj merged commit a14f093 into develop Aug 4, 2015
@weralabaj weralabaj deleted the FeatureDev_issue_194 branch August 4, 2015 08:25
@johnsimons
Copy link
Member

Actually we need to talk @weralabaj, hold on the merge

@weralabaj
Copy link
Contributor Author

@WolfByte the issue was casting to DateTime.Min when results were returned, in db they were still nulls. The only problem might be sagas, but that is out of scope.

@johnsimons
Copy link
Member

@weralabaj it looks like u missed more casts:

@johnsimons
Copy link
Member

@weralabaj can we add an acceptance test as well ?

@weralabaj
Copy link
Contributor Author

@johnsimons ok I'll revert my merge, sorry for that. Or since it's only impacting sorting (I think) is it ok to fix on develop? I didn't notice any weird behavior when doing manual tests before.

@weralabaj
Copy link
Contributor Author

@johnsimons Do I understand correctly that both omissions only change the order in which null timesent is treated, e.g. before it was casted to DateTime.Min during sorting, so it was first message in asc order, after making it nullable it'll be the last message in that sort? That's what I managed to reproduce using tests. I don't see any other impact.

@weralabaj
Copy link
Contributor Author

@johnsimons Daniel said that if we change the types then Raven MIGHT do re-indexing, it depends on how it is implemented. I need to investigate this further, so I reverted the changes from develop.
Apart from Raven forums is there anybody/anywhere I could consult?

@johnsimons
Copy link
Member

Daniel and Mauro should be able to help.
Yes, we need to be cautious with reindexing large collections.

On Tue, 4 Aug 2015 at 20:09 Weronika Łabaj notifications@github.com wrote:

@johnsimons https://github.com/johnsimons Daniel said that if we change
the types then Raven MIGHT do re-indexing, it depends on how it is
implemented. I need to investigate this further, so I reverted the changes
from develop.
Apart from Raven forums is there anybody/anywhere I could consult?


Reply to this email directly or view it on GitHub
#491 (comment)
.

John Simons
Engineer, Particular Software

Email_:_ john.simons@particular.net karen.fruchtman@particular.net
Web: www.particular.net

@johnsimons johnsimons removed this from the 1.6.0 milestone Aug 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants