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

Remove all nulls #402

Merged
merged 4 commits into from
Oct 30, 2018
Merged

Remove all nulls #402

merged 4 commits into from
Oct 30, 2018

Conversation

OlivierBlanvillain
Copy link
Contributor

@OlivierBlanvillain OlivierBlanvillain commented Oct 14, 2018

This PR refactors the entire code base to remove all usages for null as a values, and all null checks. When needed, data structures are updated to use java.util.Optional, but in a lot of cases null are removed without replacement.

The second commit re-implements nextBranch and previousBranch in functional style (best reviewed commit by commit).

We should probably branch off for v0.6 before merging this in case I missed something, but if possible I would like to avoid rebasing this for too long ^^'

Refactors the entire code base to remove all usages for null as a
values, and all null checks. When needed, data structures are updated to
use java.util.Optional, but in a lot of cases null are removed without
replacement.
Fix the Lizzie window not actually starting centered.

Remove the 0.98 factor on board size to remove the small region of
background above the board.

Fix an exception when creating a BufferedImage with width zero.
@featurecat
Copy link
Owner

wow, thanks :). I'll try to finish up all of the stuff for 0.6 now.

@zsalch
Copy link
Contributor

zsalch commented Oct 15, 2018

I'm checking.
However, there are still many PRs that do not merge. Will it be a bit early? Next, is these PR changes according to this rule, or are these PR changes during the merge?

@OlivierBlanvillain
Copy link
Contributor Author

Do you have any PR you would really like to get in before 0.6? I can focus on reviewing them this week. The fear is that if we wait for the PR queue to be empty but continue working like we do now, we will never release. I think it would be fine to release 0.6, and 0.7 in a month or something...

Next, is these PR changes according to this rule, or are these PR changes during the merge?

Either way, but I will try to keep the workload related to this PR on my shoulders. I'm happy to get everything merged first, then I will rebase this PR on top of whatever changed, or the other way around, we could merge this first and I will help fixing and updating other PRs like I did for the big one on auto formatting.

@zsalch
Copy link
Contributor

zsalch commented Oct 15, 2018

Ok, let's finish it together and I will complete the review as soon as possible. I will not submit a new PR until then.

@zsalch
Copy link
Contributor

zsalch commented Oct 16, 2018

I reviewed all the changes and there was basically no problem. There are a few minor questions, I will test it later.

@zsalch
Copy link
Contributor

zsalch commented Oct 26, 2018

@OlivierBlanvillain
If fix a few problems that have been discovered, the other looks good for me.
I have merged these changes to my source, and by using it, it seems that there are not too many problems.
I hope to merge this PR into Next earlier, so new commits and merges can be made.

@zsalch
Copy link
Contributor

zsalch commented Oct 30, 2018

LGTM.

@zsalch zsalch merged commit a510502 into featurecat:next Oct 30, 2018
@OlivierBlanvillain
Copy link
Contributor Author

@zsalch Thanks for taking the time to finish this, I'm glad it got in! Unfortunately I will have virtually no time for Lizzie for the next 10 days...

@OlivierBlanvillain OlivierBlanvillain deleted the no-null branch October 30, 2018 08:26
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.

3 participants