Skip to content

WIP: AVRO-2335 Remove Joda Time library#495

Closed
Fokko wants to merge 2 commits into
apache:masterfrom
Fokko:fd-remove-joda-time
Closed

WIP: AVRO-2335 Remove Joda Time library#495
Fokko wants to merge 2 commits into
apache:masterfrom
Fokko:fd-remove-joda-time

Conversation

@Fokko
Copy link
Copy Markdown
Contributor

@Fokko Fokko commented Mar 29, 2019

No description provided.

@probot-autolabeler probot-autolabeler Bot added build Java Pull Requests for Java binding labels Mar 29, 2019
@Fokko Fokko force-pushed the fd-remove-joda-time branch from 8bb1d53 to a50962a Compare March 29, 2019 14:35
@Fokko
Copy link
Copy Markdown
Contributor Author

Fokko commented Mar 29, 2019

Thanks, @nandorKollar I was still working on it :) Ready for review now! 👍

@Fokko Fokko requested review from dkulp, iemejia and nandorKollar March 29, 2019 21:22
@Fokko Fokko changed the title [WIP] AVRO-2335 Remove Joda Time library AVRO-2335 Remove Joda Time library Mar 29, 2019
@dkulp
Copy link
Copy Markdown
Member

dkulp commented Mar 30, 2019

This likely needs to be slit into two parts... One part is changing the various test classes to using and expecting the jsr310 types. That would definitely help make sure the jsr310 stuff is fully working and is something I would support getting in for 1.9.

The other part is the actual removal of joda-time altogether. As mentioned on the JIRA, I'm completely against the complete removal of joda-time for 1.9. 1.9 needs to continue to allow joda-time to allow a quicker migration.

@Fokko
Copy link
Copy Markdown
Contributor Author

Fokko commented Mar 31, 2019

Thanks for the feedback @dkulp

I've set this to 1.10, and won't merge this until 1.9 has been released. I fully agree with you that we can't move this into 1.9. Having jsr310 as a default 1.9 is already a breaking change. I will test in the upcoming days to get Divolte working with Avro 1.9 snapshot.

@iemejia
Copy link
Copy Markdown
Member

iemejia commented Apr 1, 2019

Big +1 on moving this to 1.10, otherwise adoption will suffer and we really need other projects to catch on Avro in particular for security issues in the dependencies that are not needed anymore in 1.9.

new String(fileInDefaultEncoding), equalTo(new String(fileInDifferentEncoding, differentEncoding)));
}

@Test
Copy link
Copy Markdown
Contributor

@nandorKollar nandorKollar Apr 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several test cases deleted, are they covered somewhere else? So far I just took a brief look at this PR, so I might have more comments later, but the changes on this test instantly caught my eyes. I'm not sure if these cases give proper coverage, but would it provide value if we wouldn't drop them, but adjust the asserts to Java date/time API instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets park this one after the 1.9 release.

@Fokko Fokko changed the title AVRO-2335 Remove Joda Time library WIP: AVRO-2335 Remove Joda Time library Apr 23, 2019
@nandorKollar
Copy link
Copy Markdown
Contributor

Gentle ping @Fokko, it seems this PR need a rebase to master. Are you still working on this?

@Fokko
Copy link
Copy Markdown
Contributor Author

Fokko commented Jul 9, 2019

@nandorKollar I'm happy to pick this up, but right now I'm extremely limited on time, feel free to take it over if you feel like it.

@Fokko
Copy link
Copy Markdown
Contributor Author

Fokko commented Aug 28, 2019

I need to revisit this later.

@Fokko Fokko closed this Aug 28, 2019
@Fokko Fokko deleted the fd-remove-joda-time branch August 28, 2019 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Java Pull Requests for Java binding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants