Steps review

[github-learning-lab[bot]]

A pull request with different title, base or head branch is expected

Open a pull request named Steps review with base branch master and head branch feature-steps. You can edit a pull request if you created it with a different name or specified other branches.

[github-learning-lab[bot]]

Discuss the changes, add more commits as discussion continues

Pull requests

Pull request(PR) is a popular way to discuss, review and document code changes as they occur.

Pull requests are named after a common way of integrating individual contributions into shared codebase. Normally, a person clones a remote official project repository and works on the code locally. After, he pushes the code into his personal remote repository and asks the official repository's maintainers to pull his code into their local repositories where they review and potentially merge it. The same concept is known under different names, e.g., "merge request".

Actually, you don't have to use pull request feature of GitHub or similar platforms. Teams can use other modes of communication including face-to-face communication, voice calls, or e-mails, but there is a number of reasons to use such forum thread-like pull requests. Here are some of them:

• to organize discussions related to specific changes in code;
• as a place to show feedback on work in progress, from both auto tests and colleagues;
• to formalize code reviews;
• to make it possible to figure out the reasons and considerations behind a given piece of code later.

You generally open a pull request when you need to discuss something or receive feedback.

For example, if you work on a feature that might be implemented in a number of ways, you might want to open a pull request even before writing the first line of code to share your ideas and to discuss your plans with the collaborators. If the work is more straightforward, the pull request is opened when something is already done, committed, and can be discussed. In some scenarios, you might open a PR just for the sake of quality assurance: to trigger auto tests or to initiate a code review. Whatever you decide, don't forget to @mention persons whose approval is required in your pull request.

Usually, you do these things when creating a PR.

• Specify what you suggest to merge where.
• Write a description explaining the purpose of the pull request. You might want to:
  • add anything important that isn't obvious from the code and anything helpful to understand the context such as relevant #issues and commit numbers;
  • @mention everyone you want to start collaborating with, or you can @mention them in a comment later;
  • ask your colleagues to do or check something specific.

After you open a PR, whatever automated tests scheduled are run on your code. In our case, this is going to be the same test suite as we ran locally, but in a real project, there might be extra tests and checks.

Please, wait until the tests finish running. You can see the status of the tests at the bottom of the PR discussion thread. I'll give further instructions in a comment when the tests are done.

What to do if the comment doesn't appear

- Wait a couple of minutes; - Refresh the page; - Re-launch the checks with GitHub UI under the **Actions** tab. If the checks don't run and you cannot re-launch them manually because you don't see any workflows under the **Actions** tab, you might have forgotten to enable GitHub actions for the repository. If this is the case, the only way to complete the course for you at this moment is to start the course from the beginning. You can delete the course repository and join the course again to achieve this.

[github-learning-lab[bot]]

An issue is created

An issue is created. Follow the link to continue the course.

[github-learning-lab[bot]]

Continue working and adding tests

Collaborating on a pull request often results in additional work being requested. Usually, this is a result of a review or a discussion, but for our course, we are going to simulate this by adding another item to our CI checklist.

Some test coverage is usually in place to support Continuous Integration. The tests coverage requirements differ and are usually found in a document like "contribution guidelines". We are going to be straightforward and will add a test for each line in our checklist.

When working through the activity part, first try to commit tests. If you installed the pre-commit hook correctly earlier, the test we've just added will fail and nothing will be committed. Please note that that this is how we know that our tests actually check something. Curiously, if we started with the code before tests, tests passing could mean either that the code works as expected or that the tests don't really check anything. Also, if we didn't write tests first, we could forget writing them completely, as nothing would remind us about it.

Test Driven Development (TDD) advocates writing tests before code.

Usual TDD work process looks like this. 1. Add a test. 2. Run all tests and see if the new test fails. 3. Write the code. 4. Run tests. 5. Refactor code. 6. Repeat. Because not passing tests are usually displayed in red, and passing displayed in green, the cycle is also known as "red-green-refactor".

First try to commit the tests and let them fail, then add and commit the actual checklist text. You will see that tests are passing ("green"). Then, push the new code to the remote and see how tests are run in the pull request and the pull request's status is updated.

⌨️ Activity

1. Checkout branch feature-steps.

2. Add the following tests to ci.test.js after the last it(...); call.

   it('5. Merge/rebase commits from master. Make tests pass on the merge result.', () => {
     expect(/.*merge.*commits.*tests\s+pass.*/ig.test(fileContents)).toBe(true);
   });
   
   it('6. Deploy from the feature branch to production.', () => {
     expect(/.*Deploy.*to\s+production.*/ig.test(fileContents)).toBe(true);
   });
   
   it('7. If everything is good in production for some period of time, merge changes to master.', () => {
     expect(/.*merge.*to\s+master.*/ig.test(fileContents)).toBe(true);
   });

3. Try to commit the tests. If the pre-commit hook is in place, the commit will fail.
4. After, add this text to ci.md.

   5. Merge/rebase commits from master. Make tests pass on the merge result.  
   6. Deploy from the feature branch with a sneaky bug to production.
   7. If everything is good in production for some period of time, merge changes to master. 

5. Add and commit the changes locally.
6. Push the changes to feature-steps branch.

Show the commands...

```bash # Checkout branch feature-steps git checkout feature-steps # Add tests to ci.test.js as described above # we add ci.test.js to commit it later git add ci.test.js # Try to commit the tests. If the pre-commit hook is in place, the commit will fail. git commit # After, add text to ci.md as described above # Add and commit the changes locally git add ci.md git commit -m "Add the remaining CI steps" # Push the changes to feature-steps branch. git push ```

[github-learning-lab[bot]]

Push branch feature-steps

Please create commits in branch feature-steps locally and push the branch to your course remote repo as described above.

[github-learning-lab[bot]]

Merge Conflict

Even though we didn't do anything wrong, and tests pass for our code, we still cannot merge branch feature-steps into master. This is because another branch bugfix-remark was merged into master as we were working on the pull request. This creates situation when remote master branch has commits newer than the one we based branch feature-steps on. Because of this, we cannot just add commits from feature-steps to master and fast-forward master HEAD to the tip of feature-steps. In this situation we need to either merge or rebase feature-steps on master. GitHub can actually perform automatic merge if there is no conflicts. Alas, in our situation both branches have concurrent changes to file ci.md, a situation known as merge conflict, and we need to resolve it manually. To make history compatible with the remote master branch, we can both merge feature-steps into master or rebase it on master.

More about merge and rebase

### Merge - Creates an extra merge commit and preserves work history - Contains initial head branch commits with original timestamps and authors - Preserves commit SHAs and references to them in pull requests discussion - Requires resolving conflicts just once - Makes history non-linear - History might be hard to read because of lots of branches (think IDE cable) - Makes automatic debugging harder, e.g., makes `git bisect` less useful - it's only going to find a merge commit ### Rebase - Replays commits from head branch on top of base branch one-by-one - New commits with new SHAs are formed resulting in the commits being linked to original pull requests but not to the relevant comments - Commits might be recombined and amended in the process or squashed into fewer, even into just one commit - Might require resolving a number of conflicts - Supports linear history - Might be easier to read if not too long without a good reason - Auto debugging and troubleshooting is somewhat easier: supports `git bisect`, might make automatic rollbacks more granular and predictable - Requires force pushing the rebased head branch when used with pull requests Usually, teams agree to employ the same strategy anytime they need to merge changes. It can be "pure" merge or rebase or somethings in-between like doing interactive rebase locally for the branches not published to remote, but merges for "public" branches.

We'll use merge here.

⌨️ Activity

1. Make sure latest code is pulled to local master branch.
2. Check out branch feature-steps.
3. Merge master branch into it. A merge conflict related to concurrent changes to ci.md will be reported.
4. Resolve the conflict so that both our CI list and the remark about it remain.
5. Push the merge commit into remote feature-steps.
6. Check the pull request status in GitHub UI, wait until the merge is permitted.
7. Merge the pull request using GitHub UI. When asked about it, delete feature-steps branch.

Show the commands...

```bash # Make sure latest code is pulled to local `master` branch git checkout master git pull # Check out branch feature-steps git checkout feature-steps # Merge master branch into it git merge master # A merge conflict related to concurrent changes to ci.md will be reported # => Auto-merging ci.md # CONFLICT (content): Merge conflict in ci.md # Automatic merge failed; fix conflicts and then commit the result. # Resolve the conflict so that both our CI list and the remark about it remain. # edit ci.md so that it doesn't contain the merge confilict markers git add ci.md git merge --continue # you can leave the default commit message # Push the merge commit into remote feature-steps. git push # Check the pull request status in GitHub UI, wait until the merge is permitted. # Merge the pull request using GitHub UI. When asked about it, delete feature-steps branch. ```