Skip to content
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

BikramSambatChronology #479

Closed
wants to merge 15 commits into from
Closed

BikramSambatChronology #479

wants to merge 15 commits into from

Conversation

grzesiek2010
Copy link

@grzesiek2010 grzesiek2010 commented Aug 10, 2018

Hi!

my name is Grzegorz and I'm an Android Developer. I work on ODK Collect where we use this project. We already implemented Ethiopic/Coptic/Islamic calendars but our users want Bikram Sambat too.

I found this fork https://github.com/bishwash72/joda-time-BIS and decided to use it. I cleaned the code and added some tests.

@bishwash72 it's your repository and I cherry-picked your commit's. Could you review this pr? It would be good if you can provide more info like:
Was your solution tested/used by real users? Why didn't you created such a pr by yourself here? etc.

Maybe you want to improve it? add anything?

@jodastephen
Copy link
Member

For licensing reasons, I'd want to hear from the original developer before considering this for inclusion. That said, I don't generally add things to Joda-Time any more, as users should be migrating to JSR-310 in Java 8 and later.

@lognaturel
Copy link
Contributor

Thanks for the response, @jodastephen!

The app we develop needs to support lower Android versions and JSR-310 is only available in API 26+. I know https://github.com/JakeWharton/ThreeTenABP is based off of your JSR-310 backport but the README suggests that there isn't a compelling reason to switch from joda-time when working on a legacy Android app. Would you recommend otherwise?

@bishwash-adhikari
Copy link

@grzesiek2010, @jodastephen, @lognaturel I have no concern related to using my source code. The reason I have developed this is to use for my Bikaram Sambat calendar App which will be opensource and published to Play store soon.

@grzesiek2010
Copy link
Author

@lognaturel I cherry-picked @bishwash72's commit (licensing) but it's at the and not before all others as you wanted because that commit contains changes in classes I refactored, renamed and there were conflicts.

@lognaturel
Copy link
Contributor

Thanks for licensing the code, @bishwash72! It would probably be simpler and more accurate for you to directly add the licenses to your own fork and then for it to be pulled in to downstream forks like this one. Either way, though, we'll need to hear from @jodastephen before knowing how to proceed.

@jodastephen any new thoughts on what our best bet is?

@jodastephen
Copy link
Member

OK, I think I'm going to have to be harsh here. Joda-Time really is a complete a finished project, and hasn't had a new calendar system for years. With Java 11 now out, the java.time.* code is well and truly in use. My goal with Joda-Time is just to keep the time-zone data up to date and nothing more.

As I said before, a PR to ThreeTen-Extra is an entirely different matter, because that will be useful going forward.

@lognaturel
Copy link
Contributor

Closing this sounds fine, @jodastephen, thanks for getting back to us.

Last question if you can remember off the top of your head -- should it be possible to define a chronology without using private methods (that is, without having to maintain a fork) or not?

@jodastephen
Copy link
Member

I thought it was, but I'm not sure anyone has ever tried. If you hit a problem with that, I'll certainly take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants