The Wayback Machine - https://web.archive.org/web/20230325020326/https://github.com/cuberite/cuberite/pull/5005
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

Jump to bottom

Removed C style arrays from the BlockEntity folder #5005

Merged
merged 6 commits into from Mar 22, 2023
Merged

Removed C style arrays from the BlockEntity folder #5005

merged 6 commits into from Mar 22, 2023

Conversation

tigerw
Copy link
Member

@tigerw tigerw commented Oct 20, 2020

No description provided.

12xx12
12xx12 requested changes Oct 25, 2020
View changes
Copy link
Member

@12xx12 12xx12 left a comment

Choose a reason for hiding this comment

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

Code looks good.

I know it's a bit nitpicky, but I think we should update the variable names to the new NamingSceme when touching the code of older functions.
If you think otherwise dismiss this review

@tigerw
Copy link
Member Author

tigerw commented Oct 28, 2020

yes I agree. Waiting on #4979 to see if I should close this or not.

12xx12
12xx12 previously requested changes Oct 29, 2022
View changes
Copy link
Member

@12xx12 12xx12 left a comment

Choose a reason for hiding this comment

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

Here I am after a long time going through pull requests.

There is another thing I saw

src/BlockEntities/BrewingstandEntity.cpp Outdated Show resolved Hide resolved
src/BlockEntities/BrewingstandEntity.cpp Outdated Show resolved Hide resolved
src/BlockEntities/BrewingstandEntity.cpp Outdated Show resolved Hide resolved
@tigerw
Copy link
Member Author

tigerw commented Nov 2, 2022

what a dereliction of duty, i've left this for two years! thanks for the comments :D

@12xx12
Copy link
Member

12xx12 commented Mar 21, 2023

Fixed it myself and updated it to master

@12xx12
Copy link
Member

12xx12 commented Mar 21, 2023

I think I crashed the CI. Again.... xD

@bearbin
Copy link
Member

bearbin commented Mar 21, 2023

Apologies for the broken CI, it's not you it's just OVH's broken openStack implementation. I really ought to sort it out, but never got the time. Maybe soon :) I have poked it so the queued up builds should be going now.

bearbin
bearbin approved these changes Mar 21, 2023
View changes
Copy link
Member

@bearbin bearbin left a comment

Choose a reason for hiding this comment

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

Fine, GTM when the build passes.

@12xx12 12xx12 merged commit c747b49 into master Mar 22, 2023
2 checks passed
@12xx12 12xx12 deleted the CArrays branch March 22, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bearbin bearbin bearbin approved these changes

@12xx12 12xx12 12xx12 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tigerw @12xx12 @bearbin @bibo38