Dead code cleanup #14894
Merged
Dead code cleanup #14894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes a bunch of unused code and removes some helpers that are available in standard Go packages these days. Signed-off-by: Dirkjan Bussink <[email protected]>
dbussink
added
Type: Internal Cleanup
Component: General
dbussink
requested review from
ajm188,
GuptaManan100,
deepthi,
mattlord,
harshit-gangal and
systay
as code owners
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vitess-bot
bot
added
NeedsBackportReason
dbussink
removed
NeedsDescriptionUpdate
This removes a trie that is not use for anything except for the tests. Also fixes some tests when colldump has been run, the path was wrong after a recent refactor for these tests to run. Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
ajm188
approved these changes
Jan 5, 2024
| @@ -17,10 +17,8 @@ limitations under the License. | |||
| package topotools | |||
|
|
|||
| import ( | |||
| "context" | |||
| @@ -58,7 +56,7 @@ func findContractedCollations(t testing.TB, unique bool) (result []CollationWith | |||
| continue | |||
| } | |||
|
|
|||
| rf, err := os.Open(fmt.Sprintf("testdata/mysqldata/%s.json", collation.Name())) | |||
| rf, err := os.Open(fmt.Sprintf("../testdata/mysqldata/%s.json", collation.Name())) | |||
There was a problem hiding this comment.
was this not working before, or maybe a VTROOT issue?
There was a problem hiding this comment.
It was not working before anymore but that was not noticed since there tests only run when you manually generate the collation data.
frouioui
reviewed
Jan 5, 2024
Comment on lines
-116
to
+112
| testMatch(t, "ContractTrie", cnt, result, nil, 2) | ||
|
|
||
| result = cwc.ContractFast.FindContextual(head, tail) | ||
| result := cwc.ContractFast.FindContextual(head, tail) |
There was a problem hiding this comment.
Why are we removing the testMatch on ContractTrie here?
frouioui
approved these changes
Jan 5, 2024
dbussink
force-pushed
the
dbussink/more-code-cleanup
branch
from
f9f4350 to
f3cfe90
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This cleans up dead code in two areas. First in
topotoolsthere's a bunch of unused code and code that can be replaced by more standard library logic.Second, we can clean up some unused code in collations. See each specific commit message for more details.
Checklist