Repository navigation
Workflow Retrospective
- Team members actively reminded each other to keep branch and commit naming aligned with the contributing guidelines, which improved consistency over the course of the project.
-
Code reviews were thorough and productive. We managed two unique reviewers
per review, and reviewers often made good catches that improved the code before
it reached
main. - The issue system worked as intended. Beyond planned work, we raised several issues to refactor and extend the codebase, which were discussed and approved by the group in meetings — this approval trail is visible in the issue history.
We initially contributed from individual forks, then moved to contributing through the organisation's fork against the upstream repo. This change stemmed from a miscommunication in the course setup about how contributions were meant to be structured, rather than a team decision we could have made differently up front.
Because the switch happened mid-development, some issues had already been reviewed and were waiting to be merged. To avoid blocking the team, we merged the already-reviewed PRs in from their individual forks, and the remaining active PRs were redirected and rebased onto the organisation's fork. This is why some commits are attributed from individual forks rather than the organisation fork.
Takeaway: the transition was handled without losing work, but it did leave the commit history split across forks. With clearer setup guidance from the outset this would have been avoided; given the miscommunication, redirecting and rebasing the open PRs was the cleanest recovery available to us.
Alongside the fork change, we hit a limitation with issue tracking. Some issues were created on the original (upstream) repo, but once we moved to our organisation's fork we could not reference or close those issues from within the fork, and GitHub did not allow us to clone the existing issues across into the fork.
As a workaround, issues that lived on our fork were closed on the original repo directly. This was another consequence of the course's fork setup rather than a team process choice. We raised it with our lecturer and attempted to get it resolved.
Takeaway: issue references and closures don't carry across a fork boundary the way branches and PRs do. If the same split setup were used again, we'd confirm where the single source of truth for issues lives before work starts, so tracking stays in one place.
SonarQube passed on the breadeadd feature branch, but once it was merged into
main the analysis failed. We also accidentally made one further push to main
after this that failed as well, before we caught it.
We were not able to fully determine the exact cause of the branch-vs-main discrepancy. On investigating the failing analysis, we found a large number of low- and medium-severity issues that, from their dates, originated in the original codebase — i.e. they pre-dated our changes — yet for some reason only surfaced in the analysis after PR #73 was merged.
Takeaway: a clean quality gate on a feature branch does not guarantee a clean
gate on main, since analysis can surface pre-existing issues from the wider
codebase once branches combine. If we ran this again we'd check the gate on main
immediately after each merge rather than trusting the branch result, and establish
a baseline of the existing codebase's issues early so pre-existing debt is clearly
separated from issues we introduce.
We assigned reviewers per branch based on availability, rather than a fixed
rota. This kept the project moving around the team's busy and differing schedules,
so PRs weren't blocked waiting on one specific person. Where possible, we also
arranged for the person who opened a PR to be the one to merge it into main once
it had been reviewed and approved.
Takeaway: availability-based assignment kept throughput up without sacrificing review quality (we still achieved two reviewers per PR). It worked well for us; a lightweight rota could make the load even more even, but the flexible approach suited a team juggling varied schedules.
Our contributing guidelines define a clear convention: branches as
<type>/<issue-number>-<short-description> (type being one of feat, fix,
docs, build, refactor, test, chore), and commit messages as a
conventional prefix + colon + brief description (e.g. docs: add contributor guidelines).
In practice we didn't follow either consistently, especially early on. Branch
names deviated from the format - for example Feat/14 record expense foreign currency and Feature/recurring expenses, which use a capitalised or invalid
type, spaces instead of hyphens, and in one case no issue number, rather than the
required feat/14-record-expense-foreign-currency form. Commit messages were a
bigger gap: not everyone realised a commit convention existed at all, so it wasn't
applied consistently until near the end of development, once the gap was spotted.
Takeaway: surface all naming rules - branch and commit - in one place at project start, and enforce them automatically rather than relying on memory. A branch-name check and a commit-message hook (or PR check) would have caught these deviations at creation time instead of leaving them in the history.
While working on ticket #11 (expense history with editing and deletion), Alyza's
feature branch fell behind upstream/main, which had since received merged changes,
including an updated database migration baseline. To catch up, she rebased the branch
after it had already been pushed to the group's fork. This caused README conflicts
that had to be resolved mid-rebase, and because the rebase gave her commits new IDs,
the local and remote branch histories diverged. The only way to update the branch and
its existing PR was to rewrite the remote history with git push --force-with-lease.
Rewriting the history of a branch that's already been pushed is risky, and we only got away with it because no one else was working on that branch and neither main branch was affected. Had a teammate branched off it or pushed to it, their work could have been overwritten or left conflicting.
Takeaway: the branch should have been synced with upstream/main earlier and more
often, so it never drifted far enough to need a large rebase and README/migration
conflicts. Rebasing before the first push, or merging upstream/main into the branch
once it was shared, would have avoided the force push entirely. When a force push is
unavoidable, --force-with-lease is the safer option since it refuses to overwrite
unexpected remote changes.