[DRAFT] RFC: naming and grammatical issue in DM

buchuitoudegou commented 2 years ago

Since the first time I read the code in DM, the issue that comments, variables, or even log info offend against the English grammar has confused me. Additionally, there is no guide about the naming, testing, or comment convention, making developing harder in particular with newcomers.

It seems we've tried resolving these issues from recent PRs by identifying illegal sentences or variables. But it's still intractable for reviewers to find out all these errors, not even to mention the historical errors. I think we probably need some tools to do the proofreading job automatically beforehand so that the reviewers save their time reviewing PRs and developers save time fixing comments. From my personal experience, Grammarly is a pretty professional and powerful proofreading tool that can run on background as a Chrome extension. VSCode now provides unofficial Grammarly for users (I don't know if it works as well as the original one) and some other similar tools, e.g. Code Spell Checker, etc. These tools save our life paying attention to these grammar bollocks and are probably able to fix those historical errors as well.

It's just my recommendation. Please let me know and leave your comments if you have any good suggestions or even objection🤭

GMHDBJD commented 2 years ago

https://github.com/pingcap/tiflow/blob/63b824a851fb102259aa5800ddc9ed4ded171565/dm/dm/config/task.go#L421 😂

buchuitoudegou commented 2 years ago

I'll record and trace grammatical log info in this issue😀

buchuitoudegou commented 2 years ago

From: https://asktug.com/t/topic/663533

https://github.com/pingcap/tiflow/blob/0edcdddf67a76ffe775288cda8e367052c75d3da/dm/dm/master/scheduler/scheduler.go#L1064

have not bound -> have not been bound.

bound is the past participle of "bind" (it's also a verb and means "leap" or "form a boundary"). No "been" might probably imply we are using its second meaning.

buchuitoudegou commented 2 years ago

From dm/syncer/checkpoint.go

https://github.com/pingcap/tiflow/blob/480fe3b3d4515eb10daf933de7075de6101cb47a/dm/syncer/checkpoint.go#L315

checkPoint -> checkpoint. It's a legal noun.