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

Vector3 is now immutable #90

Merged
merged 8 commits into from
Dec 2, 2024
Merged

Conversation

Dhaiven
Copy link

@Dhaiven Dhaiven commented Nov 19, 2023

Complete #88 just for Vector3

@dktapps
Copy link
Member

dktapps commented Feb 28, 2024

This requires a PHP 8.2 minimum

@Dhaiven
Copy link
Author

Dhaiven commented May 12, 2024

This requires a PHP 8.2 minimum

Yes but pocketmine now support PHP 8.2

@dktapps
Copy link
Member

dktapps commented May 13, 2024

Supporting 8.2 != minimum 8.2

@Dhaiven
Copy link
Author

Dhaiven commented May 13, 2024

Supporting 8.2 != minimum 8.2

When pocketmine will oblige PHP 8.2?

@dktapps
Copy link
Member

dktapps commented May 13, 2024

Not before PM6.

Copy link
Member

@dktapps dktapps left a comment

Choose a reason for hiding this comment

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

This PR is doing a ton of unnecessary changes

src/Vector3.php Outdated Show resolved Hide resolved
@Dhaiven
Copy link
Author

Dhaiven commented Dec 2, 2024

may be add in this pr immutable for Vector2, AABB... because the changes of this pr is light

@dktapps
Copy link
Member

dktapps commented Dec 2, 2024

No. Making a PR 101: Do not do multiple changes at once.

@Dhaiven
Copy link
Author

Dhaiven commented Dec 2, 2024

ok

@Dhaiven Dhaiven requested a review from dktapps December 2, 2024 15:47
@dktapps
Copy link
Member

dktapps commented Dec 2, 2024

You need to drop PHP 8.1 from composer.json and GitHub Actions

@Dhaiven
Copy link
Author

Dhaiven commented Dec 2, 2024

Like this ?

@dktapps
Copy link
Member

dktapps commented Dec 2, 2024

Yep

dktapps
dktapps previously approved these changes Dec 2, 2024
@dktapps dktapps changed the base branch from stable to major-next December 2, 2024 21:21
@dktapps dktapps dismissed their stale review December 2, 2024 21:21

The base branch was changed.

@dktapps dktapps merged commit 22f0a61 into pmmp:major-next Dec 2, 2024
2 checks passed
@Dhaiven Dhaiven deleted the immutable-vector3 branch December 3, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants