Branching

[ns-rse]

Restore divide and multiply branches.

[ns-rse]

Import ZeroDivisionError in tests for Issue 01

[SylviaWhittle]

Typo in ZeroDivisionError:

try:
    return x / y
except ZeroDivsionError as e:
    raise e("You can not divide by 0, please choose another value for 'y'.")

should be

try:
    return x / y
except ZeroDivisionError as e:
    raise e("You can not divide by 0, please choose another value for 'y'.")

[ns-rse]

Typo parametrix in one of the example tests.

[EdwinB12]

This is the code in the template. It should be arithmatic.divide(2, 0)

def test_divide_zero_division_exception() -> None:
    """Test that a ZeroDivsionError is raised by the divide() function."""
    with pytest.raises(ZeroDivisionError):
        divide(2, 0)

[SylviaWhittle]

Raise exception from e like this maybe in line with Pylint and Ruff:

    except ZeroDivisionError as e:
        raise ZeroDivisionError("Cannot divide by zero, why would you even try that?") from e

[Robadob]

The title of challenge 2 has no spacing where you switch between inline code and not. Maybe necessary to add   or similar.

Challenge 2 solution uses a bunch of parameters for git log that are not introduced previously?? It's first mentioned in a callout following that solution and not explained (significantly).

[ns-rse]

Checkout old commits with git checkout not git switch and use git log to see who made the commit.

[Robadob]

git log graph pretty callout talks about setting aliases with bash/zsh, cross platform solution is to use git config: https://git-scm.com/book/en/v2/Git-Basics-Git-Aliases

[Robadob]

Creating branches callout, missing a space after the second occurrence of main

[ns-rse]

Ensure to protect main properly to prevent repository owner from pushing and make people aware of this.

[Robadob]

Deleting branches, possibly also worth talking about git gc/git prune for cleaning up remote refs. (There are some grim shell commands which run prune, and then delete local branches that don't exist in remote refs)

[Robadob]

Challange 5 solution could also just be

git switch 585287a
git diff main tests/test_add.py

diff --git a/tests/test_add.py b/tests/test_add.py
new file mode 100644
index 0000000..bed1ffe
--- /dev/null
+++ b/tests/test_add.py
@@ -0,0 +1,5 @@
+import src.python_calculator.add as add
+
+
+def test_add():
+    assert add.add(1, 3) == 4

[ns-rse]

Be clearer/consistent about aliases gl v glod

[fredsonnenwald]

pytest fails pasting sample code out of issue template

@pytest.mark.parametrix(
    ("x", "target"),
    [
        pytest.mark(4, 2, id="square root of 4"),
        pytest.mark(9, 3.0, id="square root of 9"),
        pytest.mark(25, 5.0, id="square root of 25"),
        pytest.mark(2, 1.4142135623730951, id="square root of 2"),
    ],
)
def test_square_root(x: int | float, target: int | float) -> None:
    """Test the square_root() function."""
    assert pytest.approx(arithmetic.square_root(x), target)

seems to give errors on parametrix and mark and approx

altered code that passes pytest for me

@pytest.mark.parametrize(
    ("x", "expected"),
    [
        pytest.param(4, 2, id="square root of 4"),
        pytest.param(9, 3.0, id="square root of 9"),
        pytest.param(25, 5.0, id="square root of 25"),
        pytest.param(2, 1.4142135623730951, id="square root of 2"),
    ],
)
def test_square_root(x: int | float, expected: int | float) -> None:
    """Test the square_root() function."""
    assert arithmetic.square_root(x) == expected

[Robadob]

Challenge 6, 3rd bullet point main should be inline code.

Challenge 6, is "Ooops!" in the title intentionally a typo?

[Robadob]

Diverging branches, " changes others have made get merged into the main before you", reads as though should say main branch.

Diverging branches, Figure caption needs a line break, looks like two statements should be seperated.

[Robadob]

Rebase/merge diagrams. Might be nice to show before/after, rather than just after.

[Robadob]

Callout nano, rather than telling people to learn how to use nano. Maybe worth informing people they can use their preferred text editor.

https://docs.github.com/en/get-started/getting-started-with-git/associating-text-editors-with-git

[Robadob]

Episode is very long, it might be better broken up into ~3 episodes branching 1/2/3. Iirc carpentries recommends 3 learning objectives per episode.

https://carpentries.github.io/lesson-development-training/objectives.html#exercise-defining-objectives-for-your-lesson-20-minutes

[ns-rse]

GitKrarken allows stashes on branches.

[Romain-Thomas-Shef]

On the figures below merging and rebasing, could you extend the drawing after the merging and rebasing actions?

[ns-rse]

Add in pre-made branches have conflicts that people merge and the conflicts arise when trying to merge together.

Make each challenge independent so that people can catch up.

[ns-rse]

Emphasise the pair programming aspect of the work and that people should work through one task together then the next.

[twinkarma]

• Might be better to put all collaborative challenges at the end starting from the "Diverging branches" part
• Worktree might be too advanced? How well does it integrate with IDE?
  • In terms of supporting stashes on multiple branches, would it be better to recommend committing then amend instead?

[twinkarma]

• Merging vs rebase - if you merge from main into your feature branch insted of rebase, github PR will show diff for other files that's been merged in and not just the changes in your feature branch, this makes code review harder.