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

fix 15848, cherry-pick #16435 #16436

Merged
merged 2 commits into from
May 28, 2024
Merged

Conversation

jiangxinmeng1
Copy link
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #15848

What this PR does / why we need it:

Not set object created by CN as 1PC node

@mergify mergify bot requested a review from sukki37 May 28, 2024 02:49
@mergify mergify bot added the kind/bug Something isn't working label May 28, 2024
@matrix-meow matrix-meow added the size/XS Denotes a PR that changes [1, 9] lines label May 28, 2024
@matrix-meow
Copy link
Contributor

@jiangxinmeng1 Thanks for your contributions!

Pull Request Review:

Title:

The title of the pull request is clear and concise, mentioning the fix number and cherry-pick reference.

Body:

The body of the pull request provides essential information about the type of PR (BUG fix), the specific issue it addresses (issue #15848), and a brief description of what the PR does. It would be beneficial to have a more detailed explanation of the issue and the reason for the change.

Changes:

  1. In the table_space.go file, within the prepareApplyObjectStats function:
    • Line 250: The change modifies the CreateNonAppendableObject function call by setting the second argument to false instead of true.

Problems Identified:

  1. Potential Bug Introduction:
    • The change made in the pull request modifies the argument passed to CreateNonAppendableObject from true to false. This alteration may have unintended consequences as it changes the behavior of object creation. It is crucial to ensure that this change does not introduce new bugs or issues related to object creation.

Suggestions for Improvement:

  1. Clarify Intent:

    • It is recommended to provide a more detailed explanation in the pull request description about why the change from true to false is necessary. Understanding the rationale behind the modification can help reviewers and maintainers assess the impact of the change accurately.
  2. Testing:

    • To mitigate the risk of introducing bugs, thorough testing should be conducted after making this change. Specifically, test cases related to object creation and functionality should be executed to ensure that the modification does not adversely affect the system.
  3. Code Review:

    • A more comprehensive code review by team members familiar with the object creation logic can help identify any potential issues or improvements related to this change.
  4. Documentation Update:

    • Update any relevant documentation or comments in the code to reflect the change made in the CreateNonAppendableObject function call. This will help future developers understand the reasoning behind the modification.

Overall Impression:

The pull request addresses a specific bug fix related to object creation in the table_space.go file. However, it is essential to ensure that the change does not introduce new issues and that the rationale behind the modification is well-documented for clarity. Conducting thorough testing and involving relevant team members in the review process can enhance the quality and reliability of the codebase.

@mergify mergify bot merged commit d1033a2 into matrixorigin:1.2-dev May 28, 2024
17 of 19 checks passed
@aylei aylei mentioned this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working size/XS Denotes a PR that changes [1, 9] lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants