Re: [PATCH v4 3/9] checkout.[ch]: move struct declaration into *.h
On Fri, Jun 01 2018, Junio C Hamano wrote: > Thomas Gummerer writes: > >> We seem to have plenty of structs defined in '.c' files, if they are >> only needed there. Its use also seems to be single purpose for the >> callback data, so I'm a bit puzzled how having this in a header file >> instead of the .c file would be helpful? >> >> I feel like having only the "public" part in the header file also >> helps developers that are just looking for documentation of the >> functions they are looking at, by having less things to go through, >> that they wouldn't necessarily care about. > > Yup, sounds like a sensible criterion to choose between <*.h> and > <*.c>. Yes this makes sense. Thanks both. I misunderstood the idiom. Makes sense in hindsight (& with both of your advices) only to put structs in the *.h if it's actually used outside of that API. Will move this around in v5.
Re: [PATCH v4 3/9] checkout.[ch]: move struct declaration into *.h
Thomas Gummerer writes: > We seem to have plenty of structs defined in '.c' files, if they are > only needed there. Its use also seems to be single purpose for the > callback data, so I'm a bit puzzled how having this in a header file > instead of the .c file would be helpful? > > I feel like having only the "public" part in the header file also > helps developers that are just looking for documentation of the > functions they are looking at, by having less things to go through, > that they wouldn't necessarily care about. Yup, sounds like a sensible criterion to choose between <*.h> and <*.c>.
Re: [PATCH v4 3/9] checkout.[ch]: move struct declaration into *.h
On 05/31, Ævar Arnfjörð Bjarmason wrote:
> Move the tracking_name_data struct used in checkout.c into its
> corresponding header file. This wasn't done in 7c85a87c54 ("checkout:
> factor out functions to new lib file", 2017-11-26) when checkout.[ch]
> were created, and is more consistent with the rest of the codebase.
We seem to have plenty of structs defined in '.c' files, if they are
only needed there. Its use also seems to be single purpose for the
callback data, so I'm a bit puzzled how having this in a header file
instead of the .c file would be helpful?
I feel like having only the "public" part in the header file also
helps developers that are just looking for documentation of the
functions they are looking at, by having less things to go through,
that they wouldn't necessarily care about.
> Signed-off-by: Ævar Arnfjörð Bjarmason
> ---
> checkout.c | 7 ---
> checkout.h | 7 +++
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/checkout.c b/checkout.c
> index bdefc888ba..8d68f75ad1 100644
> --- a/checkout.c
> +++ b/checkout.c
> @@ -3,13 +3,6 @@
> #include "refspec.h"
> #include "checkout.h"
>
> -struct tracking_name_data {
> - /* const */ char *src_ref;
> - char *dst_ref;
> - struct object_id *dst_oid;
> - int unique;
> -};
> -
> static int check_tracking_name(struct remote *remote, void *cb_data)
> {
> struct tracking_name_data *cb = cb_data;
> diff --git a/checkout.h b/checkout.h
> index 4cd4cd1c23..04b52f9ffe 100644
> --- a/checkout.h
> +++ b/checkout.h
> @@ -3,6 +3,13 @@
>
> #include "cache.h"
>
> +struct tracking_name_data {
> + /* const */ char *src_ref;
> + char *dst_ref;
> + struct object_id *dst_oid;
> + int unique;
> +};
> +
> /*
> * Check if the branch name uniquely matches a branch name on a remote
> * tracking branch. Return the name of the remote if such a branch
> --
> 2.17.0.290.gded63e768a
>
[PATCH v4 3/9] checkout.[ch]: move struct declaration into *.h
Move the tracking_name_data struct used in checkout.c into its
corresponding header file. This wasn't done in 7c85a87c54 ("checkout:
factor out functions to new lib file", 2017-11-26) when checkout.[ch]
were created, and is more consistent with the rest of the codebase.
Signed-off-by: Ævar Arnfjörð Bjarmason
---
checkout.c | 7 ---
checkout.h | 7 +++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/checkout.c b/checkout.c
index bdefc888ba..8d68f75ad1 100644
--- a/checkout.c
+++ b/checkout.c
@@ -3,13 +3,6 @@
#include "refspec.h"
#include "checkout.h"
-struct tracking_name_data {
- /* const */ char *src_ref;
- char *dst_ref;
- struct object_id *dst_oid;
- int unique;
-};
-
static int check_tracking_name(struct remote *remote, void *cb_data)
{
struct tracking_name_data *cb = cb_data;
diff --git a/checkout.h b/checkout.h
index 4cd4cd1c23..04b52f9ffe 100644
--- a/checkout.h
+++ b/checkout.h
@@ -3,6 +3,13 @@
#include "cache.h"
+struct tracking_name_data {
+ /* const */ char *src_ref;
+ char *dst_ref;
+ struct object_id *dst_oid;
+ int unique;
+};
+
/*
* Check if the branch name uniquely matches a branch name on a remote
* tracking branch. Return the name of the remote if such a branch
--
2.17.0.290.gded63e768a
