-
Notifications
You must be signed in to change notification settings - Fork 662
Writing a Good Pull Request
TODO: Add links to styleguides
-
🦘 RE: style guide section, I think its hard to set a style guide for example code as different ecosystems have different patterns, Codelabs too, it's markdown, little style guidance there. For plugins, definitely worth mentioning that we abide by the Google JavaScript style guide: https://google.github.io/styleguide/jsguide.html
-
❕ That's a good point, especially about the examples. It would be really hard to come up with something that is broadly applicable.
For the codelabs I was thinking about linking to the Writing Tips section. While it's not exactly a style guide I think it's still a good idea for people to read through it. Does that sound like it would work?
And for plugins I think how we're going to do that is still a little bit up in the air. We definitely want to tell people we follow the jsguide! But we also need to put how to handle private apis... somewhere lol. I'm not sure what would be the best solution for right now. But it seems like Rachel might have wanted to talk that one over with you guys first.
[Edit: Rachel beat me to it haha]
-
🐙 In the page on writing a codelab I wrote a short section on writing style, said code should follow the google style guide, and added a link to Technical Writing One so that we don't have to rewrite everything they've said about good technical writing.
-
❕ Ok I've updated that section with all the links we currently have available. =)
-
-
🐙 Suggestion: put the list of steps in "Get Set Up" into a collapsed/detail section. That makes the page read more smoothly, while still keeping the information easily accessible.
- ❕ Good idea! That section kind of annoyed me cuz it was so bold, but only important for certain people. The dropdown makes it much better.
Pull requests are like the life blood of a repository. They keep everything healthy and moving. This page details how to create a PR that is complete and easy to review, which makes it more likely that your PR will get merged!
Here are steps you can take to make sure you create the best PR possible.
Before you jump in and start writing code, it's helpful to communicate with the core team so they know what you're interested in.
If there is an issue you are interested in, which is in your jurisdiction and has the correct status, the communication is simple! Just put a comment on the issue saying you're going to start to work on it. This makes sure that we don't have multiple people working on the same thing.
If you have an idea which is not covered by an issue, please write one up before you begin work. This gives the team a chance to discuss how best to build out the change before you start building, which saves you work in the long run.
If you've contributed to a GitHub repository before, the process for getting set up will be familiar to you. It's important to note that in this repo we work off of the master branch. This is different than the Blockly core repo, where work is done off of develop.
If you haven't contributed to a GitHub project before, you can use these steps to get started:
Steps
- Fork and clone the blockly-samples repository. GitHub has a wonderful tutorial about forking a repo. To apply it to blockly-samples, just replace every instance octocat/Spoon-Knife with google/blockly-samples.
- Sync your fork with this repo. GitHub provides a tutorial for syncing a fork as well.
-
Create a new branch by running the below command in a terminal. The name should help you remember what you're working on.
git checkout -b myBranchName - Make your changes. Be sure to follow the rest of the recommendations in this doc!
-
Save your changes. You can use the below commands to commit your work.
git commit -am "My commit message" git push origin myBranchName
Always try to keep your changes small and focused. We would much rather review multiple smaller PRs than review one giant PR. Some good rules of thumb are:
- Fix one problem. Don't try to tackle multiple issues at once.
- Limit the scope. Usually a PR should take < 8hrs (depending on your familiarity with the codebase).
- Use commits. If your PR feels a little big, split the changes into logical groups using git commits.
Why care about code style? We're in it for the long term, and consistent style makes maintenance easier. Style refers to how you name your variables, but also covers how you structure your code, write comments, and more. Where possible we use tools such as eslint to automate style checks.
Be sure to read the style guide for your specific category of project before you get to work:
- [Plugin style guide]
- [Example style guide]
- Codelab style guide
If you are writing a plugin you should also read through the Guidelines on Using Blockly APIs.
And if you are writing any JavaScript please follow the Google JavaScript style guide.
Before you put up a PR you should always test that your changes are working, so that you don't have to go back and fix things later. Here are some ideas for testing the different categories of projects:
- For plugins: write automated mocha tests covering your changes.
- For examples: manually test all of your demonstrated functionality.
- For codelabs: run through the entire tutorial in a clean environment and test any example code you provide.
This is the last and arguably the most important part of creating a PR! Writing up the summary.
Writing a great PR summary helps other developers review your changes, making it more likely that it will get accepted faster!
Your summary should include things like:
- What issue your PR is related to.
- What change your PR adds.
- How you tested your change.
- Anything you want reviewers to scrutinize.
- Any other information you think reviewers need.
If you follow the PR template when your create your request you should be good to go! Just remember to be as concise and complete as possible.
Happy Coding!