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 apparent contradiction in vector.h header #100147

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

esainane
Copy link
Contributor

@esainane esainane commented Dec 7, 2024

3205a92 was a major commit which removed PoolVector, and replaced most references to PoolVector with Vector instead. In most cases, this was appropriate, given that PoolVector was being replaced with Vector, as an effective generalist in 64-bit address space layouts.

However, vector.h itself was left with an artifact advising the reader to use Vector instead of Vector for large arrays. While this led to a fascinating deep dive, and hopefully improved some of the documentation along the way, it's probably best to clean this up for the next person.

@esainane esainane requested a review from a team as a code owner December 7, 2024 16:24
@clayjohn clayjohn modified the milestones: 4.x, 4.4 Dec 8, 2024
@clayjohn
Copy link
Member

clayjohn commented Dec 8, 2024

Looks great! The final step before merging is to squash all the commits together so that the whole PR only contains 1 big commit with all your changes. We like to merge one commit at a time to keep the git history clean and navigable.

If you don't know how to do that, we have a helpful tutorial in the official documentation https://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#the-interactive-rebase

3205a92 was a major commit which removed `PoolVector`, and replaced
most references to `PoolVector` with `Vector` instead. In most cases,
this was appropriate, given that `PoolVector` was being replaced with
`Vector`, as an effective generalist in 64-bit address space layouts.

However, vector.h itself was left with an artifact advising the reader
to use `Vector` instead of `Vector` for large arrays. While this led
to a fascinating deep dive, and hopefully improved some of the
documentation along the way, it's probably best to clean this up for
the next person.
@esainane
Copy link
Contributor Author

Done!

@Repiteo Repiteo merged commit d108d4f into godotengine:master Dec 10, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 10, 2024

Thanks!

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

Successfully merging this pull request may close these issues.

6 participants