Skip to content

Conversation

@timvaillancourt
Copy link
Collaborator

@timvaillancourt timvaillancourt commented Jun 5, 2022

Description

This PR moves 2 x blocks of code to use switch statements for a bit more clarity, as suggested by the gocritic linter. Also the .NewGoMySQLReader() func of go/binlog/gomysql_reader.go was simplified (unused err removed, etc)

This change should result in the exact same behaviour as before

Details:

  1. Simplify .NewGoMySQLReader() func in go/binlog/gomysql_reader.go
    • Remove unused err return
  2. Use a type-based switch for binlog event type in go/binlog/gomysql_reader.go
    • This simplifies a future PR for GTID support
  3. Use switch on the CutOverType field to pick cutover logic
  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors

@timvaillancourt timvaillancourt self-assigned this Jun 5, 2022
@timvaillancourt timvaillancourt added this to the v1.1.5 milestone Jun 5, 2022
@timvaillancourt timvaillancourt requested review from a user and rashiq June 5, 2022 22:03
@timvaillancourt timvaillancourt changed the title Use switch statements for readability Use switch statements for readability, simplify .NewGoMySQLReader() Jun 9, 2022
@timvaillancourt timvaillancourt merged commit 0b066c1 into master Jul 6, 2022
@timvaillancourt timvaillancourt deleted the simplify-switch-stmts branch July 6, 2022 21:45
ghost pushed a commit that referenced this pull request Jul 7, 2022
…)` (#1135)

* Use `switch` statements for readability

* Simplify initBinlogReader()
@ghost ghost mentioned this pull request Jul 7, 2022
ghost pushed a commit that referenced this pull request Jul 7, 2022
…)` (#1135)

* Use `switch` statements for readability

* Simplify initBinlogReader()
RainbowDashy pushed a commit to RainbowDashy/gh-ost that referenced this pull request Jul 11, 2022
…)` (github#1135)

* Use `switch` statements for readability

* Simplify initBinlogReader()
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.

4 participants