![]() No law of demeter violations (providing whole objects to methods when all that's needed is the value of one attribute of them)?. ![]() Are all files within a reasonable size (less than 100 loc)?.no fields/methods made public that aren't used externally? Is everything as private as possible, i.e.Are all classes/methods/variables named properly so that the code is self-describing?.Do all classes have only one responsibility?.model code in a model, controller logic in a controller, app-specific helper code in a helper, generic helper code in a library? Is every piece of code in the right place, i.e.When a PR gets LGTMed ("looks good to me"), it will be merged by the author, and the feature branch should be deleted from both the local machine as well as from Github.If your reviews usually take several rounds, try to be more thorough before sending off your PRs. Well written codebases get reviewed in one iteration (one set of comments, one round of fixes, good to go).Getting comments on your PR is good, it means you are alive and learning. No comments on a PR means the review was not thorough enough.Mark pull requests that should not be merged as "WIP" in the PR title ("WIP: new settings page"). ![]() You can use multiple reviewers for different types of code.
0 Comments
Leave a Reply. |
AuthorWrite something about yourself. No need to be fancy, just an overview. ArchivesCategories |