Fix: Downgrade reflections to 0.9.11 (fixes #60)

[porunov]

Fixes #60

[freemo]

@porunov ahh wonderful, includes a unit test. perfect. I'll review tonight most likely maybe tomorrow (11:30pm here). Glad to see you added yourself as a contributor as well and the changelog, looks good at first glance just need to test it locally.

[porunov]

@freemo Thank you! The unit test should fail with reflections 0.9.12 and succeed with reflections 0.9.11.

[freemo]

@freemo Thank you! The unit test should fail with reflections 0.9.12 and succeed with reflections 0.9.11.

Perfect, thanks.

By the way I can and will approve pull requests over here at github, but it requires extra steps from me (I need to check it out and recreate the PR over on gitlab). If you plan to be a regular contributor it would really help if you could submit the pull requests on our gitlab instance instead. However if that will be a barrier that prevents you from contributing feel free to continue to contribute them here.

[porunov]

@freemo Sure. I have created a PR in GitLab here: https://git.qoto.org/Ferma/Ferma/-/merge_requests/1

It is my first usage of GitLab, so if I do something wrong, just tell me

[freemo]

I think its good, thanks. I'll review in more detail shortly.

On Sat, Mar 7, 2020 at 1:50 AM Oleksandr Porunov notifications@github.com wrote:

@freemo https://github.com/freemo Sure. I have created a PR in GitLab here: https://git.qoto.org/Ferma/Ferma/-/merge_requests/1

It is my first usage of GitLab, so if I do something wrong, just tell me

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Syncleus/Ferma/pull/61?email_source=notifications&email_token=AAXESARJD6N2H3YYNIFRRLTRGGK4DA5CNFSM4LDJBHW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEODJOAQ#issuecomment-596023042, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXESAVNDIJOK3J5ONR3W53RGGK4DANCNFSM4LDJBHWQ .

[porunov]

@freemo I was also researching other options yesterday. I noticed the next reflections fork: https://github.com/aschoerk/reflections8 Current latest version is 0.11.7: https://search.maven.org/search?q=net.oneandone.reflections8 It says: Since org.reflections 0.9.12 is released: obsolete. But the thing is that 0.11.7 version of reflections8 is newer then 0.9.11 of reflections. I noticed that they don't have such bug (didn't test it yet but the code of Reflections.class looks fine to me). Similar to 0.9.12 is no longer depends on guava but depends on java 8. So, I was thinking, we might give a try for 0.11.7 of reflections8 and after 0.9.13 of reflections is released, switch back to reflections (assuming it fixes the bug).

What do you think about it?

[freemo]

That sounds like an OK solution to me. honestly its a temporary solution and i dont think will be very critical either way so long as it works. IF that gives the problem it looks like a version bump so what could be the problem?

On Sat, Mar 7, 2020 at 9:07 PM Oleksandr Porunov notifications@github.com wrote:

@freemo https://github.com/freemo I was also researching other options yesterday. I noticed the next reflections fork: https://github.com/aschoerk/reflections8 Current latest version is 0.11.7: https://search.maven.org/search?q=net.oneandone.reflections8 It says: Since org.reflections 0.9.12 is released: obsolete. But the thing is that 0.11.7 version of reflections8 is newer then 0.9.11 of reflections. I noticed that they don't have such bug (didn't test it yet but the code of Reflections.class looks fine to me). Similar to 0.9.12 is no longer depends on guava but depends on java 8. So, I was thinking, we might give a try for 0.11.7 of reflections8 and after 0.9.13 of reflections is released, switch back to reflections (assuming it fixes the bug).

What do you think about it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Syncleus/Ferma/pull/61?email_source=notifications&email_token=AAXESAVUMTW2XPRLIVT3VXTRGKSPZA5CNFSM4LDJBHW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOEEIOA#issuecomment-596132920, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXESASYVU4ICSVCYCGQPW3RGKSPZANCNFSM4LDJBHWQ .

[porunov]

@freemo Overall it isn't a big problem because we can exclude transitive dependencies but it would be good to exclude guava from dependencies because it often becomes a problem when you have several libraries which both compiled with using different guava versions and that is why they might be not compatible because guava versions are often incompatible with older guava versions. I checked 0.11.7 of reflections8 it works as expected and doesn't introduce guava dependency (same as 0.9.12 of reflections). I will check how much Ferma depends on guava, maybe we can move to use Java 8 instead of guava. If so, I will provide the PR shortly

[freemo]

I think we directly depend on Guava. If we remove it from our dependencies then it would break Ferma except when Guava is explicitly included in addition to it. Which I dont think is a good solution.

By the way im pulling down your PRs now and reviewing them. Will have the bulk of them in shortly.

On Sat, Mar 7, 2020 at 11:32 PM Oleksandr Porunov notifications@github.com wrote:

@freemo https://github.com/freemo Overall it isn't a big problem because we can exclude transitive dependencies but it would be good to exclude guava from dependencies because it often becomes a problem when you have several libraries which both compiled with using different guava versions and that is why they might be not compatible because guava versions are often incompatible with older guava versions. I checked 0.11.7 of reflections8 it works as expected and doesn't introduce guava dependency (same as 0.9.12 of reflections). I will check how much Ferma depends on guava, maybe we can move to use Java 8 instead of guava. If so, I will provide the PR shortly

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Syncleus/Ferma/pull/61?email_source=notifications&email_token=AAXESAVGYHOUP5D2IBBHOYTRGLDPLA5CNFSM4LDJBHW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOEG7NI#issuecomment-596144053, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXESAR74LQM4ZFS4CEUEADRGLDPLANCNFSM4LDJBHWQ .

[porunov]

@freemo I know that Ferma directly depends on guava but are you OK if we move all guava implementations to java 8 implementations (i.e. streams API and direct java utils usage)? I think it isn't hard to detach Ferma from guava. After that we can just remove guava dependency from Ferma. Or do you have another point of view regarding the guava (i.e. like cleaner code at some places or something like that)?

[porunov]

I am just asking you, because I have noticed that many libraries (datastax cassandra driver, reflections, etc.) are moving out from guava due to dependency problems. It is much easier to include dependency which doesn't depend on guava then dependency which depends on guava due to binary incompatibility between many guava versions.

[freemo]

I am perfectly ok moving it all to java8 yes. As long as there are no unexpected issues that arise from that that I cant foresee its good by me.

On Sun, Mar 8, 2020 at 12:05 AM Oleksandr Porunov notifications@github.com wrote:

@freemo https://github.com/freemo I know that Ferma directly depends on guava but are you OK if we move all guava implementations to java 8 implementations (i.e. streams API and direct java utils usage)? I think it isn't hard to detach Ferma from guava. After that we can just remove guava dependency from Ferma. Or do you have another point of view regarding the guava (i.e. like cleaner code at some places or something like that)?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Syncleus/Ferma/pull/61?email_source=notifications&email_token=AAXESAVJSJFGXUAHLXZE6L3RGLHM5A5CNFSM4LDJBHW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOEHSII#issuecomment-596146465, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXESASW75DYBWWSHVW7XQLRGLHM5ANCNFSM4LDJBHWQ .

[freemo]

Yea at the time that I wrote Ferma things were different, java8 didnt exist. I completely support the move.

On Sun, Mar 8, 2020 at 12:08 AM Oleksandr Porunov notifications@github.com wrote:

I am just asking you, because I have noticed that many libraries (datastax cassandra driver, reflections, etc.) are moving out from guava due to dependency problems. It is much easier to include dependency which doesn't depend on guava then dependency which depends on guava due to binary incompatibility between many guava versions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Syncleus/Ferma/pull/61?email_source=notifications&email_token=AAXESATMUVHBRSISL6FO66TRGLHXVA5CNFSM4LDJBHW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOEHT2I#issuecomment-596146665, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXESARHQQM5XHLXKZOD6ALRGLHXVANCNFSM4LDJBHWQ .

[porunov]

Suppressed by #63