Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions common-content/en/module/legacy/practices-to-remember/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,39 @@ Some of our standard development practices are _even more important_ when workin
We should be _consistent_ with our formatting. We may not have any control over what decisions were made in the past. When we're writing brand new code in a new code, we can choose whether we use two spaces or four. But when we jump into an existing codebase, we should use whatever style it already has. If we send a PR fixing a small bug, and it changes every line in the file, it's really hard to see what changed, and what stayed the same. If we have two people joining a codebase, and one is reformatting every file they touch to use two spaces, and the other is reformatting every file they touch to use two spaces, there are going to be lots of spurious changes, and lots of merge conflicts.\

To approach this, we may need to:

* Disable our auto code formatters
* Change the configuration of our auto code formatters (maybe by finding an existing configuration in the repo, or adding one)
* Reformat the whole repo with a consistent configuration in one commit, and then keep it formatted.

We should **never** include spurious reformatting changes in the same commit as real code changes.

### Clean PRs

We should (almost!) always work from a clean copy of main. When you move from one task to the next, do not keep the changes from the previous task. Often in the real world your last PR will be merged to main before you branch for the new task, so you will have the changes included in the second task's PR, but they are not part of the branch, they are there as part of main. When doing these exercises (or in real world scenarios where your previous branch hasn't been merged) it is important to start from a clean checkout of main.

Ideally you'll think about this immediately after you submit the previous PR and before you make any changes. In which case the clean route is

```plain
git switch -c new-branch main
```

which will create a new one based on main, and switch to it.

If you've got random uncommitted changes in your local folder, they will not be removed by the above command. So before you do that command you can clean things up:

```plain
git reset --hard origin/main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a useful "I already made my branch wrong I need to fix it" command, but I read your text above as suggesting this is for creating a new branch?

Do we want to suggest:

git switch --create new-branch main

as a default way to make a new branch, and give the reset command as an "If you already created your branch and need to undo things" command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Further commit incoming

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I read this, this risks people resetting their old branch to origin/main rather than "the committed tip of the branch being updated"?

Suggested change
git reset --hard origin/main
git reset --hard HEAD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well yes, but since they're about to make a new branch from main, this is what we want isn't it? I'm assuming we want each PR to be based on main?

```

If you realise (as often happens) too late, and are trying to clean up after you've created your new branch and you have your task2 changes as well as task1 changes, then if they are in different folders you can reset the task1 folder. Check the git history for the last commit to main and note the commit hash. Then do

```plain
git restore --source=COMMIT_HASH --worktree task1folder
```

Whenever you commit, have a look at the changed files. Check they are only what you need for this task.

### PR Descriptions

PR descriptions are even more important when working with Legacy Code. We probably had to do a lot of learning to work out how something works, what was wrong, and how to fix out. The person reviewing our change may not know the codebase very well either. The more detail we can write in our PR descriptions, the more information the next person has when they need to investigate something (and the easier it will be for our reviewer to review our change).
Expand Down
Loading