Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "=?utf-8?Q?Micha=C5=82?= Lowas-Rzechonek" 
        <michal.lowas-rzechonek@silvair.com>
Date:   Tue, 18 Jun 2019 09:35:59 +0200
To:     Inga Stotland <inga.stotland@intel.com>
Cc:     linux-bluetooth@vger.kernel.org, brian.gix@intel.com
Subject: Re: [PATCH BlueZ] mesh: Clean up includes
Message-ID: <20190618073559.tmpaj2oellkf5354@mlowasrzechonek2133>
Mail-Followup-To: Inga Stotland <inga.stotland@intel.com>,
        linux-bluetooth@vger.kernel.org, brian.gix@intel.com
References: <20190617213847.21705-1-inga.stotland@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190617213847.21705-1-inga.stotland@intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Inga,

On 06/17, Inga Stotland wrote:
> This adds #include for json-c/json.h in mesh-db.h and removes this
> include from the other files that don't need to reference json-c.

While I agree about removal from cfgmod-server, model and node, I don't
think we should remove the include from storage and move it to mesh-db.

I'd rather see #includes follow https://include-what-you-use.org/
approach, that is:

    (...) for every symbol (type, function variable, or macro) that you
    use in foo.cc, either foo.cc or foo.h should #include a .h file that
    exports the declaration of that symbol.

Moreover, I think headers should only be included as-needed (mostly in
.c files), and headers should contain forward declarations of various
types, to make them opaque.

Such an approach cuts implicit dependencies, making code maintenance
easier: when, for example, someone decides to refactor mesh-db not to
use json_object anymore, they shouldn't need to suddenly deal with
missing declarations in other modules (in this case, storage).

Moreover, each header should be self-sufficient: in order to use API
exposed by a given header, it should be enough to include that header
only. At the moment, this is not the case.

A minor side-effect would be faster builds and less rebuilds when code
is being worked on.

If you agree, I am more than happy to fix this throughout the mesh/
codebase.


regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND