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

[Do Not Merge] Documentation, Small Redesigns, Migration to Gradle, & More #2

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AlbertSnows
Copy link

Do not merge this PR, as it contains too many changes to review at once. It does, however, pass all tests and builds. As such, this PR should not introduce any breaking changes.

Why does this PR exists?

Since this repo is largely educational, this PR exists to bring more attention to some redesign and documentation considerations that more formally align with java standards. At the whims of the repo owner, any part of this PR can be integrated into the main repo. Otherwise, if this repo is to remain untouched, my hope is that this PR will draw others to look at the contributions I've added which will, hopefully, make some concepts and design choices a bit clearer.
Additionally, there are some parts of the dev environment that either don't work well anymore, have depreciated components, or are otherwise out of date. This PR also attempts to fix these. As for gradle, I just have less headaches using it, so I decided to migrate the repo over. It's purely a personal preference.
Finally, if the repo owner has any requests, critiques, or considerations I would be happy to address them. I made these changes of my own accord, and figured someone else might find them useful. I'm not particularly attached to any of my changes, and I view any feedback as a boon.

Thank you!

Pair<List<S>, List<F>> successesAndFailures = results.stream().collect(split());
if(successesAndFailures.right.isEmpty()) {
return Result.success(successesAndFailures.left);
if(successesAndFailures.right().isEmpty()) {
Copy link
Author

Choose a reason for hiding this comment

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

stuff like this is from converting to a record


public final L left;
public final R right;
public record Pair<L, R>(L left, R right) {
Copy link
Author

Choose a reason for hiding this comment

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

If it was converted to a record, it should be because the only fields were public final, and thus shouldn't cause any issues (assuming property access was changed from A.B to A.B())

@AlbertSnows
Copy link
Author

I have briefly reviewed the PR. The changes are largely documentation or QoL in nature, with a few different renames, changes, converts.

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.

1 participant