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

Kernel: Clean up duplicated code #24551

Merged
merged 1 commit into from
Jun 9, 2024

Conversation

brody-qq
Copy link
Contributor

@brody-qq brody-qq commented Jun 9, 2024

There are 2 methods in InodeVMObject that are almost identical. This commit makes them both use the same code path.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 9, 2024
@BertalanD
Copy link
Member

nit: "cleanup" is a noun, "clean up" is the verb.

@brody-qq brody-qq force-pushed the release-clean-pages-cleanup branch from ca29480 to 7713ddf Compare June 9, 2024 11:40
@brody-qq brody-qq changed the title Kernel: Cleanup duplicated code Kernel: Clean up duplicated code Jun 9, 2024
@brody-qq
Copy link
Contributor Author

brody-qq commented Jun 9, 2024

nit: "cleanup" is a noun, "clean up" is the verb.

ok, I changed the titles for the PR and commit 👍

Copy link
Member

@supercomputer7 supercomputer7 left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

Small nitpick: I'd change the commit title to be:
Kernel/Memory: Use try_release_clean_pages when releasing all clean pages

or something similar...

The methods try_release_clean_pages() and release_all_clean_pages() in
InodeVMObject are almost identical. This commit makes them both use the
same code path.
@brody-qq brody-qq force-pushed the release-clean-pages-cleanup branch from 7713ddf to 5057d31 Compare June 9, 2024 15:44
@brody-qq
Copy link
Contributor Author

brody-qq commented Jun 9, 2024

Thanks for the feedback

I changed the commit title and message to be a bit more specific

@nico nico merged commit 5058873 into SerenityOS:master Jun 9, 2024
10 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 9, 2024
@brody-qq brody-qq deleted the release-clean-pages-cleanup branch June 9, 2024 20:59
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.

None yet

4 participants