by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
Roger Howell
@MysterAitch
Just seeing if it's easy enough to get tests running on java 15
should I interpret the question as a nudge to do it sooner rather than later? :wink:
Harry Freeborough
@hfreeb
no rush :P
Maarten Coene
@maartenc
Any chance my 2 PR's could get merged too ?
Roger Howell
@MysterAitch
@maartenc -- likely yes :smile:
I think the last time I looked you'd mentioned they're not quite ready to be merged but I didn't look back #sorry !
do you have the PR #s for them?
Roger Howell
@MysterAitch
scratch that, saw them in the search results :)
Maarten Coene
@maartenc
:)
just noticed a merge conflict in one of them
Roger Howell
@MysterAitch
probably my fault :wink:
#2674 -- the first post says to not merge yet due to an issue with generics, is this still the case?

#2687 -- no excuse, I skipped over it sorry!
Maarten Coene
@maartenc
#2674 was ok, but fails now after merging master into it... I'll have a look (but probably not today, so you can skip that one)

#2687 seems already fixed by merging #2675
Roger Howell
@MysterAitch
sorry, my bad -- hadn't intended to step on your toes!
1 reply
Roger Howell
@MysterAitch
so.. I'm all done barring @maartenc's PRs -- hopefully I'll stop pulling the rug out from under your feet now :wink:
hmm... I'm not convinced all these github actions are needed on every change (and they take a while), but they are quite nice to have!
Roger Howell
@MysterAitch

#2674 was ok, but fails now after merging master into it... I'll have a look (but probably not today, so you can skip that one)

Just commented the same, but yes this seems to be me -- getTypeDeclaration() now returns optional which would explain the errors in the test results

I'm happy to tweak it to get it into this release, but if you have it open and would like to push an update that's where I would suggest looking
Danny van Bruggen
@matozoid
Wall of text :-D Anything I need to say something about? :-)
Roger Howell
@MysterAitch
Just nod and agree :wink:
Only thing I would appreciate a pointer on is #2685 -- how to read the input to the parser without consuming the whole stream then forwarding on a brand new provider... I don't understand the unicode preprocessor!
If what is there already is "good enough" then that could ALSO be included within this bumper-sized release :grinning:
Danny van Bruggen
@matozoid
Yeah, I think you can simply register it as a pre- and post-processor. Look through the stream while preprocessing, store the eol info inside the processor, and in the post-processor you have the result object so you can store the eol info wherever you're storing it currently.
Roger Howell
@MysterAitch
ohhhhhhhh....... for some reason I though process was a static method thus there's no way to pass state.... that actually helps a lot, thanks!! :D
Danny van Bruggen
@matozoid
The only difficult thing is handling the JavaCC Provider implementation :-# That interface is begging for one-off errors.
Roger Howell
@MysterAitch
Release pushed out and tweet posted!! :grinning:
Harry Freeborough
@hfreeb
oops
just tested the release, and it seems like I didn't fully test what I had previously when approving the pr to be merged
In MethodResolutionLogic#isApplicable
ResolvedType expectedTypeWithInference = methodUsage.getParamType(i);
The expectedTypeWithInference part causes an IndexOutOfBoundsException, I think it should be ResolvedType expectedTypeWithInference = expectedArgumentType;
I should have added more test cases really
Roger Howell
@MysterAitch
Oops haha, if you can get a tested PR in quick we can sneak it in before maven central catches up ;)
Roger Howell
@MysterAitch
or perhaps you could share some code that triggers this issue?
Harry Freeborough
@hfreeb
I've done the fix, I'll PR it within a few minutes
Roger Howell
@MysterAitch
Haha, I was about to say I'm headed off to bed 😅😅
Will keep an eye out for it -- ideally with a test that fails without the fix please 😊😊
Harry Freeborough
@hfreeb
ok done
Roger Howell
@MysterAitch
looks good :)
Roger Howell
@MysterAitch
release replaced -- caught it in time :)
my bad re: muddling up the i vs j though haha.. thank you for catching it so quickly @hfreeb ! :D
Actually time for sleep now though! 😴🥱🥱
Danny van Bruggen
@matozoid
Release replaced? :-O I hope that works :-)
Harry Freeborough
@hfreeb
hmm, doesn't look like it did work :P what I see on maven central is the original 3.16.0 (dated 2020-05-24 20:13)
I guess might as well wait until next release for the change now
Roger Howell
@MysterAitch
Was worth a try :wink:
I suspect this checkbox prevented it:
image.png
That said, bintray indicated the sync was successful... so, who knows?
Roger Howell
@MysterAitch
3.16.1 released just now anyway to head off any issues :)
Maarten Coene
@maartenc
Thanks @MysterAitch :+1:
Roger Howell
@MysterAitch
thank you to you @maartenc and @hfreeb and others too -- can't do it without you! :D