Open Source Development 5 - Action Without Pull Req
References for this Part
Brasseur, V. M. Forge Your Future with Open Source 1st ed., Pragmatic Bookshelf, 2018
(Brasseur, V. M., 2018, chapter 6-7)
Model Solutions Previous Lessons
Today
Some Words on CW1 and CW2
- There was some misunderstanding regarding the courseworks. The rumors about oral examination, we beleive, concerns re-exams from the previous semester.
- CW1 which is published on the website already is coding and testing of code. To be handed in as a repos with code and comments in a README.
- Re CW2: We are checking the rules as layed out in the MID. It will be published next week.
Vicky’s Book Chapter 6 - Make a Difference
some quotes:
Review Contributions
Reviewing contributions is one way to contribute without committing a single file to version control, but it likely will require you to use version control to retrieve and view the contributions up for review.
We’ll go through code reviews later in this chapter as a separate topic, but all types of contributions can benefit from another set of eyes on them. Reviewing a contribution helps locate defects early on in the process, when it’s still relatively easy to fix them. It also helps to confirm that the work that was committed is actually the work that needs to be done. Very often either the issue is vague in describing the requirements or the contributor misun- derstands them somehow. Catching these problems during review helps to get development back on track sooner. …
and later
About Providing Feedback
We talked about receiving feedback in Make a Contribution. This is where we get to talk about the other side of that same coin: providing feedback.
If I tell you that something you did in your contribution is “stupid” or “naive,” how would you feel? You’d probably be angry, hurt, or both, and rightfully so. These are mean-spirited words that when directed at people, can cut like knives. Words matter, and they matter a great deal. Therefore, put as much thought into the words you use when leaving feedback for a contribution as you do into any other form of contribution you give to the project. As you compose your feedback, think to yourself, “How would I feel if someone said this to me? Is there some way someone might take this another way, a less helpful way?” If the answer to that last question has even the chance of being a yes, backtrack and rewrite your feedback. It’s better to spend a little time rewriting now than to spend a lot of time apologizing later. …
and later again:
What to Look for in a Code Review
If you do decide to review code contributions, what kind of things should you look for? The answer, as you probably expect, is “it depends on the project.” That said, there are several things you can keep in mind regardless of the project, the code, or the programming language being used. While it may seem like these tips are only for people earlier in their programming career, nothing could be further from the truth. What follows are best practices for code reviews by people of any experience level. Whether you’re a neophyte or a master, these tips can help you spot potential problems in any code review.
Does the code even pass the build? Does the project use continuous integration/continuous deployment (CI/CD) or otherwise have its test suite run automatically? If the test suite doesn’t pass after the code contribution… You’re a smart person. I don’t need to tell you that this is a big red flag that something may be wrong with that code. Politely ask the contributor to study the build/test errors and correct them before you continue to invest your time in reviewing the contribution.
Is the code even readable? You don’t have to be an expert in a programming language to tell whether the code is readable. Strange loops, short and vague variable and function names, inconsistent use of whitespace or brackets, large blocks of commented out code… Many things could make a piece of code difficult to read, but the end result is the same: unreadable code is unmaintainable code.
Do one thing and do it well. It’s best practice that each class, method, or function in a program do one thing and do it well. This reduces the complexity of any one piece of code, making it shorter and much easier to understand, maintain, and test. Be on the lookout for any piece of code that’s overloaded and trying to do too many things. A good clue for this can be that the code has a complicated or long conditional statement.
The DRY Principle: Don’t Repeat Yourself. Is there any code that occurs more than once, even if it’s doing similar but not entirely identical things? If so, it should be refactored out into a separate class, method, or function. Repeated code means changes have to be applied in multiple places, leading to a higher chance of error. Plus, refactoring it out can make it easier to test.
How is the error handling? Are errors explicitly handled? Are they even handled at all? Do errors include descriptive messages or are they vague, “an error has occurred”-type things? Proper error handling doesn’t only make debugging a lot easier; it improves the experience for everyone who uses the program.
Is the code efficient? More advanced programmers will have an easier time of determining this for a new piece of code, but even new programmers can have a feeling for whether code appears unwieldy, or whether it looks like it’s working harder than it should to accomplish what it does. If your instinct tells you that the code may not be efficient, it may be worth flagging for more explanation.
How is the test coverage? Does the code come with any tests? Both unit and integration? If the code was covered by existing tests, were they updated to make sure they’re still valid? If it’s new code or there were no tests before, did the author add any? If there are tests, don’t forget to review those as well as the rest of the code.
Does the code actually do what it’s supposed to? If the code is intended to add a feature or fix a bug, compare the code against the issue it’s supposed to close to make sure the code does what’s expected of it. It’s very easy to misunderstand expected functionality, forget to include a piece, or include more than is expected (more is not always better).
Is the code documented? Code comments, installation instructions, user docs, API docs, troubleshooting docs… There are so many different ways a piece of code could or should be documented. Because documentation is so difficult, yet so important, it’s usually easier to do it piecemeal as each new feature or bug fix is added to the repository. If the code you’re reviewing doesn’t come with changes to the documentation, you may want to suggest the author add some to help avoid the technical and usability debt that can accrue by skipping documentation.
As you can see, while knowing about code is very helpful when doing code review, there are a lot of things you can see and provide feedback on even if you’re just getting started with programming. If the project is supportive of it, even less experienced programmers can provide a lot of valuable insights while also learning more about the project code and how it all fits together.
Vicky’s Book Chapter 7 - Interact with the Community
some quotes:
You’ve overcome all the roadblocks in your way, you’ve followed all instructions to the letter, and you’ve finally submitted your first contribution.
Congratulations!
In free and open source software, the only feeling more amazing than making your first contribution is having it accepted as part of the project. When and how does that happen? You probably won’t be surprised to hear the answer is, “It Depends.” It’ll depend on the project itself, your contribution, whether anyone is available to review your contribution, and any number of other vague and mysterious variables.
After Your First Contribution
Once you submit your contribution, you must practice this little thing we like to call Patience. There are any number of reasons that it might take a while for the project members to get around to reviewing your contribution. …
Exercises
No new exercises today. You continue work on exercises from OSD.4