[PATCH v2] commit-graph: remove a duplicate assignment

[PATCH v2] commit-graph: remove a duplicate assignment

[Alex Henrie]

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 commit-graph.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commit-graph.c b/commit-graph.c
index 9b02d2c426..d0e1f9e1f2 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1522,7 +1522,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
 
 static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
 {
-	struct commit_graph *g = ctx->r->objects->commit_graph;
+	struct commit_graph *g;
 	uint32_t num_commits = ctx->commits.nr;
 	uint32_t i;
 
-- 
2.23.0

Re: [PATCH v2] commit-graph: remove a duplicate assignment

[Johannes Schindelin]

Hi,


On Thu, 26 Sep 2019, Alex Henrie wrote:

> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>

Hrm, the commit message should reflect Stolee's careful consideration,
no? An empty commit message won't be helpful e.g. when cherry-picking on
top of a branch that already has your v1.

Ciao,
Johannes

> ---
>  commit-graph.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 9b02d2c426..d0e1f9e1f2 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1522,7 +1522,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
>
>  static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
>  {
> -	struct commit_graph *g = ctx->r->objects->commit_graph;
> +	struct commit_graph *g;
>  	uint32_t num_commits = ctx->commits.nr;
>  	uint32_t i;
>
> --
> 2.23.0
>
>

Re: [PATCH v2] commit-graph: remove a duplicate assignment

[Derrick Stolee]

On 9/27/2019 8:21 AM, Johannes Schindelin wrote:
> Hi,
> 
> 
> On Thu, 26 Sep 2019, Alex Henrie wrote:
> 
>> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> 
> Hrm, the commit message should reflect Stolee's careful consideration,
> no? An empty commit message won't be helpful e.g. when cherry-picking on
> top of a branch that already has your v1.

I agree that we need more in the commit messages here. It is
helpful to include _why_ you are doing this. Keep in mind
that your cover letter is not recorded with the commit at all.

Further, when updating a patch series, you should re-roll the
entire series as a v2 (preferably as a reply to the original
series). Keep in mind that this patch should NOT be applied
on top of your other series because then 'g' is not assigned
at all. Mechanically, it COULD apply, but it would break
in tests.

Patches 2 and 3 in your series also have feedback that would
be good to apply in your v2.

Thanks,
-Stolee