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

Tighten up nulls #104

Merged
merged 14 commits into from Oct 17, 2022
Merged

Tighten up nulls #104

merged 14 commits into from Oct 17, 2022

Conversation

prdoyle
Copy link
Contributor

@prdoyle prdoyle commented Sep 21, 2022

Allow some, disallow others.

Supersedes #100. Fixes #54.

@prdoyle
Copy link
Contributor Author

prdoyle commented Sep 21, 2022

@hrldcpr - I've attempted to make the commits logical to help you review these changes. If you review commit-by-commit it should all make sense.

@hrldcpr
Copy link
Owner

hrldcpr commented Sep 22, 2022

Awesome thanks! I'm a bit busy for the next several days, but I hope to look this over soon

@hrldcpr
Copy link
Owner

hrldcpr commented Sep 28, 2022

(still busy, until next week, but haven't forgotten)

@prdoyle
Copy link
Contributor Author

prdoyle commented Oct 16, 2022

Hey @hrldcpr! Just checking in to see whether you're still planning to do a release with all these new goodies. OrderedPMap in particular would make the library much handier for my project.

If it's going to take more than a week or two, I'll just merge my changes using the prior version of pcollections, and upgrade once the new release is available.

@hrldcpr
Copy link
Owner

hrldcpr commented Oct 17, 2022

Sorry for the delay, this is great, thank you! I added a few small javadoc edits but other than that I think it's ready to merge.

Thanks for the tip to go commit-by-commit, that was a nice way to review it.

My one thought is that I'm tempted to make ConsPStack support nulls, I'll take a quick stab at that but then we can (finally) release 4.0.0!

@hrldcpr hrldcpr merged commit 4cd8636 into hrldcpr:master Oct 17, 2022
@hrldcpr
Copy link
Owner

hrldcpr commented Oct 17, 2022

(Oh and great call swapping in requireNonNull, much cleaner 🧘 )

@prdoyle
Copy link
Contributor Author

prdoyle commented Oct 18, 2022

Thanks @hrldcpr! Also nice catch on all those javadoc bugs.

@prdoyle prdoyle deleted the tighten-up-nulls branch October 18, 2022 00:34
@hrldcpr
Copy link
Owner

hrldcpr commented Oct 18, 2022

No problem! And I added some more null value testing, which uncovered a few subtle bugs - 0848dae

I'm gonna try to add some similar null value testing for TreePMap tomorrow, because I have a feeling it might have some similar bugs - #106

Other than that we should be able to do a release soon, sorry for the delay. Though come to think of it I could just release all the pre-null stuff as 4.0.0 and then finish up testing the null stuff in no hurry and release it as 5.0.0…

@hrldcpr
Copy link
Owner

hrldcpr commented Oct 18, 2022

I got nulls working in the few remaining places, and am releasing v4.0.0 now!

For some reason it takes a while to propagate to Maven Central, but should be there by tomorrow morning, lemme know if there's any issues

@hrldcpr
Copy link
Owner

hrldcpr commented Oct 18, 2022

@prdoyle 4.0.0 is live, it seems https://central.sonatype.dev/artifact/org.pcollections/pcollections/4.0.0

@prdoyle
Copy link
Contributor Author

prdoyle commented Oct 22, 2022

Thanks so much! I've brought it into my project now and it was super simple.

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

Successfully merging this pull request may close these issues.

fail consistently for null keys/values
2 participants