-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Date / time handling. #15
Comments
I'm pretty familiar with JodaTime - which I asssume is pretty similar to NodaTime. Anything I can do to jump in? I'm a bit rusty at C# but I'd love to take a crack at something on this. |
That'd be really good, thanks Jason. What do you need from me? I've been through the date / time code and fixed a few bugs where I've found them, plus placed as many unit tests as I can think of for date / time parsing. It has some fields and properties that I think can be reduced but I'd like to get more tests around that code before doing that. So perhaps a straight replace initially? As longs as the tests carry on passing, it should be good. Happy to help you set up anything you need as well. |
I've had a look around and I've got some ideas to discuss, it's opened up a bit of a can of worms but all looks fun and workable.
My feeling is that each of these should be a separate issue and commit to keep them small and workable. If you have good experience of handling these data types then perhaps you can help on the others too? Happy for you to, but not assuming anything. Thanks again. |
Wow - these are things that I'd never even considered considering before. (Does that even make sense?) I'm currently working on figuring out how to get the solution to run well inside VS Code on Mac OS, but I'm going to have access to full blown Visual Studio here soon, so if I'm not able to make much progress using .Net core, it'll be OK. I'll let you know where I want to jump in first. I don't want to actually touch anything until I'm sure I can get the tests running right :-) |
That's good. I think we'll manage to get it ported to .Net core this week, just need to get some bits straight in my head first. That should make it accessible to you on OSX. I'll try it on OSX as well when that's done. |
Hey @RyanONeill1970 and anyone else interested. I just attempted to pull in NodaTime latest stable release (1.3.2) and it says it's incompatible with .Net Core. Should I try a newer release to see if it's incompatible? They all have -alpha in the release version... |
2.0.0-alpha20160729 seems compatible. But leaving in Alpha seems like a bad idea. |
Assigned this one to myself. Lemme know if anyone else wants to help crack it open. |
I can help you, if you like. |
@pichardoSoftware I'm currently looking at refactoring the GedcomDate class to using NodaTime for comparison operations, etc. If you wanted to pick up a different class, we could merge your changes into the issue-15 branch. Just let me know. |
@lowenthal-jason I'll will revise the implementations and let you know where am I going to start, thanks for letting me help. |
So to my understatement the intention is to change all base DateTime implementations to NodaTime, if that is right most of them are based on the GedcomDate properties, I could go on changing the tests. |
@pichardoSoftware sounds like you might end up clashing with the changes by @lowenthal-jason , I'll leave it up to you chaps to sort out. As you say @pichardoSoftware , if you wait until @lowenthal-jason commits his changes (or puts them on a new branch) then you could add to the tests then. Thank you both for getting involved. |
A lot has changed so I'll close this and split it out to smaller tasks as needed unless anyone has anything they want to commit (no problem if you don't, just giving you time to respond in case you do). |
I like what I see of the NodaTime project and I'd like to figure out if we will gain any benefit from implementing this in GedcomDate and throughout the project.
The text was updated successfully, but these errors were encountered: