Leaderboard
Popular Content
Showing content with the highest reputation on 09/16/18 in all areas
-
1 pointContribute to open source and get a free limited-edition T-shirt What's Hacktoberfest? Hacktoberfest — brought to you by DigitalOcean in partnership with GitHub and Twilio — is a month-long celebration of open source software. Maintainers are invited to guide would-be contributors towards issues that will help move the project forward, and contributors get the opportunity to give back to both projects they like, and ones they've just discovered. No contribution is too small—bug fixes and documentation updates are valid ways of participating. From October 1 to October 31, contribute to any open source project (Hercules included) on GitHub to get a free T-shirt! 5 pull requests are required. (Pull requests do not have to be merged and accepted; as long as they've been opened between the very start of October 1 and the very end of October 31, they count towards a free T-shirt.) We will be marking easy-to-tackle issues with the Hacktoberfest tag so that first time contributors can more easily find them. >> Register on hacktoberfest.digitalocean.com Resources GitHub Learning Lab How to create a Pull Request on GitHub Understanding the GitHub Flow Open source 101 Hercules documentation Hercules wiki FAQ It is free to participate? Yes! Is shipping included? Yes. DigitalOcean offers free worldwide shipping. What shirt sizes are available for Hacktoberfest 2018? DigitalOcean have not yet made public the size chart for 2018, but we know they at least offer S to 4XL sizes, for both male and female. What's included in the package? A thank you letter. A T-Shirt. A bunch of cool stickers. Do I need to register for Hacktoberfest before starting to open Pull Requests? No. You may register at any time during the month of October and DigitalOcean will count your pull requests retroactively from October 1 onwards. Do all of my Pull Requests have to be sent to the same repository? No. You may send PRs to any number of repositories you like, and as long as they are public and have an OSI-approved license they will count towards the 5+ PRs objective. Do I have to wait for the start of October to open Pull Requests? You may contribute all year long, but only PRs that are opened during the month of October will be counted.
-
1 pointAbout Code Review and Why You'd Want Your Code to Be Reviewed Hello, fellow developers and code contributors! As you certainly know, years ago, Hercules adopted a workflow based on pull requests, that includes code review as one of the necessary steps before any new piece of code makes it into the master branch of the repository. While being an uncommon and somewhat controversial change in Hercules (and in the RO emulator scene in general), code review is part of the workflow of most software projects, both open source and closed source, and has many benefits. Why Code Review The benefits of code review are several: "Given enough eyeballs, all bugs are shallow" [Linus's Law by Eric S. Raymond -- The Cathedral and the Bazaar, 1999]. While the law is not strictly true, it's certainly true that the more developers read and analyze a piece of code, the more likely it is that bugs that might be hidden in it are discovered early. Testing is not enough. It's very hard (or in the case of our codebase just plain impossible) to cover all the possible edge cases when testing a new feature or a fix. An additional pair of eyes reading the code may help discovering those more easily. This includes cases where the client would normally prevent a certain thing from happening, but it's not ensured anywhere on the server side. Better quality of code. By having other developers read a piece of code, they'll end up wondering why a certain approach was taken, rather than another, and discuss it with the submitter, leading to better, more efficient algorithms, or better engineered code. Better documentation. Since the code needs to be read by other people, it'll require proper comments (or they'll ask for explanations about the parts they can't easily explain). This increases the chance that the author, or anyone else that will need to read the same code again months or years after it's been submitted, will be able to understand it again, by finding appropriate comments in the appropriate parts of the code. Better insight into the code across the team. By reading code from different parts of the emulator as part of the review process, every team member increases their own general knowledge of the software, bit by bit. This is a very efficient way of learning how different parts of the emulator work, and why they were implemented that way. Future-proofing. By having public reviews, we keep a permanent trace of what were the hot topics and why certain decisions were taken, when a certain part of the emulator was implemented. If a bug arises, or something needs to be redesigned in future, we can look up the associated pull request and related discussion, and learn more about the discussion that went on in the past, and what's hiding behind code design decisions. Reviewing code from other people, as well as having one's own code reviewed by others might not be easy for everyone, especially at the beginning, but please try your best. Here are some suggestions on how to approach code review from either side. How to approach code review (for code authors) As a code author, the worst thing you can do is to be afraid or shy about other people judging your code. This is the wrong approach! Don't be shy, have your code looked at by others, have them praise you for your genial approach to tackle a problem, listen to their suggestions on how to improve it. But be ready to defend your implementation, if you believe it's better than the suggestions you receive, or if the critics that are moved against it are wrong or meaningless. Always keep in mind that: Having your code reviewed and commented on isn't humiliating. Other people are spending their time looking at your code, asking you why you did something in a certain way rather than another, suggesting improvements. Both sides have a lot to learn from each others. (On the other hand, if no one reviews your code, that's somewhat humiliating!) If someone spots an issue in your code, it doesn't mean that you're a bad developer. We all make mistakes, and we should be happy to learn from them (and it's definitely better if someone spots them and points them out to us before it's too late and they were able to do some harm). Never, ever, take code review personally. No one will laugh about you, fire you, kill you, shame you, etc. if your code is commented on. If you believe you're right and the comments you received are pointless or wrong, chance is that you really are right. Be ready to defend your reasons, it's possible that the reviewer didn't think of them. It is your duty to explain them your reasons. How to approach code review (for code reviewers) Reviewing code is several orders of magnitude harder than having your own code reviewed. You have to check the code for several classes of problems, point out your findings, suggest improvements. And you still have to deal with the worry about hurting the code author's feelings when pointing out a mistake. Here are some things you should keep in mind when reading and reviewing code from other people: You're not judging a person. You're judging code. Don't make your review sound personal. Always think of uncommon and edge cases, and never assume they can't happen, unless there's an explicit check that makes them impossible to happen. Even if the code was tested by the author, it doesn't mean that it can't cause problems to other existing features, or have some issues the author couldn't think of. If the same person writes and tests a piece of code, the chance that they don't test the cases they forgot to handle while coding, is very close to 100%. If the code is not following the project's style guidelines (and this isn't just about indentation, but also about names, conventions about function calls, proper modularization, etc), it is your duty to point it out now, before it's merged. This will make the life of your fellow developers easier later on. Think defensively. Consider the code you have in front of you as buggy until you can prove its correctness. If you see that a sanity check is missing, ask the author to add it. If you believe that a function returns the wrong value in certain cases, even if very unlikely to occur, prepare an example of input for which that happens and point it out. Remember that threats such as overflows, underflows, buffer overruns, null pointers, invalid pointers, numeric (floating point) approximation, etc. are always behind the corner, check for them as often as possible and prove that they can't occur. And remember that, while the code author isn't your enemy (and code review shouldn't generate negative feelings), it's often a good idea to think of them as your "professional enemies". There's a chance that something nasty is hiding in their code, even if they didn't write it with ill intent, and as such, you shouldn't blindly trust the code, regardless of who the author is. Don't be afraid when you comment on other people's code. Your goal isn't to hurt their feelings, you're asking them for explanations and/or suggesting the way you would have done something. Likewise, don't be afraid of making a pointless comment. If the author has a good reason for their implementation, be ready to take back your comment and learn from them. Don't accept compromises. If you're firmly convinced that the author's defense of their code is wrong, your duty is to prove them wrong. But if they manage to convince you, don't be ashamed of admitting you were wrong. Happy reviewing!