*: renaming variables/structs to improve readability / decrease confusion #49989

mjonss · 2024-01-02T22:26:12Z

What problem does this PR solve?

Issue Number: close #49988

Problem Summary:
Naming of variables/structs causes confusion, since the names are not matching the type.

What changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- Rename of variables only, no logic change.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

….PartitionInfo

tiprow · 2024-01-02T22:26:31Z

Hi @mjonss. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

codecov · 2024-01-02T22:37:53Z

Codecov Report

Merging #49989 (e0ba603) into master (4c7102c) will decrease coverage by 12.0211%.
The diff coverage is 92.6605%.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #49989         +/-   ##
=================================================
- Coverage   79.3291%   67.3080%   -12.0211%     
=================================================
  Files          2447       2558        +111     
  Lines        673702     839883     +166181     
=================================================
+ Hits         534442     565309      +30867     
- Misses       117888     250844     +132956     
- Partials      21372      23730       +2358

Flag	Coverage Δ
integration	`36.6352% <77.9816%> (?)`
unit	`79.3014% <89.9082%> (-0.0277%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`73.6130% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`71.8734% <ø> (+3.3761%)`	⬆️

YangKeao

LGTM

Defined2014 · 2024-01-03T08:03:20Z

/retest

Defined2014 · 2024-01-03T08:03:41Z

/run-check-issue-triage-complete

tiprow · 2024-01-03T08:03:42Z

@Defined2014: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Defined2014 · 2024-01-03T08:07:31Z

pkg/planner/core/point_get_plan.go

@@ -315,7 +315,7 @@ type BatchPointGetPlan struct {
 	dbName           string
 	TblInfo          *model.TableInfo
 	IndexInfo        *model.IndexInfo
-	PartitionInfos   []*model.PartitionDefinition
+	PartitionDefs    []*model.PartitionDefinition


Why name it PartitionDefs? This is different from PointGet.

It's better to use PartitionDefs or Partitions?

I now also changed PointGetPlan, to be partitionDef, so it is consistent.

ti-chi-bot · 2024-01-05T09:10:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Defined2014, YangKeao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Defined2014,YangKeao]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-01-05T09:10:19Z

[LGTM Timeline notifier]

Timeline:

2024-01-03 07:45:26.659046194 +0000 UTC m=+2243017.696273114: ☑️ agreed by YangKeao.
2024-01-05 09:10:17.967581634 +0000 UTC m=+2407.551835321: ☑️ agreed by Defined2014.

Defined2014 · 2024-01-05T09:35:49Z

/retest

tiprow · 2024-01-05T09:36:11Z

@Defined2014: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mjonss · 2024-01-05T09:58:27Z

/test build

tiprow · 2024-01-05T09:58:49Z

@mjonss: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test build

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mjonss · 2024-01-05T10:00:02Z

/ok-to-test

mjonss · 2024-01-05T10:43:53Z

/test tiprow_fast_test

ti-chi-bot · 2024-01-05T10:43:56Z

@mjonss: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test pull-integration-ddl-test
/test pull-lightning-integration-test
/test pull-mysql-client-test
/test unit-test

The following commands are available to trigger optional jobs:

/test canary-notify-when-compatibility-sections-changed
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-br-integration-test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-integration-nodejs-test
/test pull-sqllogic-test
/test pull-tiflash-test

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test
pingcap/tidb/pull_integration_ddl_test
pingcap/tidb/pull_mysql_client_test

In response to this:

/test tiprow_fast_test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mjonss · 2024-01-05T17:18:50Z

/retest

tiprow · 2024-01-05T18:01:09Z

@mjonss: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
tiprow_fast_test	`e0ba603`	link	true	`/test tiprow_fast_test`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

hawkingrei · 2024-01-06T02:30:41Z

/test all

…sion (pingcap#49989) close pingcap#49988

mjonss added 2 commits January 2, 2024 21:44

Renamed PartitionInfo to [Phys]PlanPartInfo to distinguish from model…

5757002

….PartitionInfo

Renamed partInfo to partDef/partition where it was actually a partDef

93c3a6f

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 2, 2024

mjonss added a commit to mjonss/tidb that referenced this pull request Jan 3, 2024

Removed renaming of variables due to pingcap#49989

b012bdb

mjonss added a commit to mjonss/tidb that referenced this pull request Jan 3, 2024

Reverted name change of partDef/Info due to pingcap#49989.

3340e87

YangKeao approved these changes Jan 3, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 3, 2024

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Jan 3, 2024

Defined2014 reviewed Jan 3, 2024

View reviewed changes

Updated more struct variable names according to reviews.

d34e4da

Defined2014 approved these changes Jan 5, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 5, 2024

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Jan 5, 2024

Merge remote-tracking branch 'pingcap/master' into planPartInfo-rename

e0ba603

ti-chi-bot bot merged commit 9b0fd9e into pingcap:master Jan 6, 2024
19 of 20 checks passed

mjonss deleted the planPartInfo-rename branch January 7, 2024 16:01

AilinKid pushed a commit to AilinKid/tidb that referenced this pull request Jan 17, 2024

*: renaming variables/structs to improve readability / decrease confu…

bfecca6

…sion (pingcap#49989) close pingcap#49988

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: renaming variables/structs to improve readability / decrease confusion #49989

*: renaming variables/structs to improve readability / decrease confusion #49989

mjonss commented Jan 2, 2024

tiprow bot commented Jan 2, 2024

codecov bot commented Jan 2, 2024 •

edited

Loading

YangKeao left a comment

Defined2014 commented Jan 3, 2024

Defined2014 commented Jan 3, 2024

tiprow bot commented Jan 3, 2024

Defined2014 Jan 3, 2024 •

edited

Loading

Defined2014 Jan 3, 2024 •

edited

Loading

mjonss Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

Defined2014 commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

mjonss commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

mjonss commented Jan 5, 2024

mjonss commented Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

mjonss commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

hawkingrei commented Jan 6, 2024

*: renaming variables/structs to improve readability / decrease confusion #49989

*: renaming variables/structs to improve readability / decrease confusion #49989

Conversation

mjonss commented Jan 2, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Jan 2, 2024

codecov bot commented Jan 2, 2024 • edited Loading

Codecov Report

YangKeao left a comment

Choose a reason for hiding this comment

Defined2014 commented Jan 3, 2024

Defined2014 commented Jan 3, 2024

tiprow bot commented Jan 3, 2024

Defined2014 Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Defined2014 Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

mjonss Jan 5, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

[LGTM Timeline notifier]

Defined2014 commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

mjonss commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

mjonss commented Jan 5, 2024

mjonss commented Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

mjonss commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

hawkingrei commented Jan 6, 2024

codecov bot commented Jan 2, 2024 •

edited

Loading

Defined2014 Jan 3, 2024 •

edited

Loading

Defined2014 Jan 3, 2024 •

edited

Loading