On Tue, Feb 07 2023, Calvin Wan wrote:
> [...]
> + sps->result = 1;
> + strbuf_addf(err,
> + _(status_porcelain_start_error),
> + task->path);
> + return 0;
> [...]
> + if (retvalue) {
> + sps->result = 1;
> + strbuf_addf(err,
> + _(status_porcelain_fail_error),
> + task->path);
> [...]
This is nitpicky, but what's with the short lines and over-wrapping?
If you change these two to (just using my macro version on top, but it's
the same with yours):
strbuf_addf(err, _(STATUS_PORCELAIN_START_ERROR), task->path);
And:
strbuf_addf(err, _(STATUS_PORCELAIN_FAIL_ERROR), task->path);
Both of these are under our usual line limit at their respective
indentation (the latter at 77, rule of thumb is to wrap at 79-80).
> + if (submodules.nr > 0) {
Don't compare unsigned to >0, just use "submodules.nr".
> + int parallel_jobs;
nit: add extra \n, or maybe just call this "int v", as it's clear from
the scope what it's about...
> + if (git_config_get_int("submodule.diffjobs", ¶llel_jobs))
> + parallel_jobs = 1;
> + else if (!parallel_jobs)
> + parallel_jobs = online_cpus();
> + else if (parallel_jobs < 0)
> + die(_("submodule.diffjobs cannot be negative"));
Can't you use the "ulong" instead of "int" and have it handle this "is
negative?" error check for you?
> +
> + if (get_submodules_status(&submodules, parallel_jobs))
> + die(_("submodule status failed"));
> + for (size_t i = 0; i < submodules.nr; i++) {
Another case that can use for_each_string_list_item().
> +struct submodule_parallel_status {
> + size_t index_count;
> + int result;
> +
> + struct string_list *submodule_names;
> +
> + /* Pending statuses by OIDs */
> + struct status_task **oid_status_tasks;
> + int oid_status_tasks_nr, oid_status_tasks_alloc;
For new structs, let's use size_t, not "int" for alloc/nr.
Also, as this is 7/7 and we're not adding another such pattern for the
forseeable future, can we just call these "size_t nr", "size_t alloc"
and "tasks"?
And having said all that, it turns out this is just dead code that can
be removed? Blindly copied from submodule_parallel_fetch?
