Re: [PATCH spice-streaming-agent v2 1/2] Don't tag the plugin interface as 1.0 just yet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 16 Nov 2017, at 11:43, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> 
>> 
>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>> 
>> This patch series introduces changes in the ABI and API, and I expect
>> a few more to arrive shortly. So I tagged the ABI version as 0.01,
>> and all 0.x versions are considered as incompatible with one another
>> by default. When we reach ABI and API stability, we can bump to a
>> non-zero version number and then we will need to preserve ABI and API
>> compatibility within a major version moving forward.
>> 
>> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
> 
> Still nack, I already explained why.

The first “explanation" was this:

> Nack... ABI not compatible

I addressed that by adding a comment explaining why (in addition to my mail replies).

Then you added:

> Going backward? Would not be easier to get to 1.1 instead?

But by your compatibility rules, 1.1 should be forward compatible with 1.0. Which
is not the case here.

Then you wrote
> I would try to extend maintaining API/ABI if possible. As far as I can see
> your mainly concerns are lack of some features.
> 

I replied to this.

You then replied to Victor but not to me, so I took that as meaning that my last
explanation as to why I wanted to clean up the ABI right now had convinced you.


So I’m confused. Are you:

- Convinced we can only move the ABI number forward, i.e. the next one has to be 2.0. If so, why?

- Convinced that what I want to do should be done in an ABI or API comptaible way. If so, how?


Thanks
Christophe


> 
>> ---
>> include/spice-streaming-agent/plugin.hpp | 2 +-
>> src/concrete-agent.cpp                   | 3 +++
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/spice-streaming-agent/plugin.hpp
>> b/include/spice-streaming-agent/plugin.hpp
>> index 727cb3b..607fabf 100644
>> --- a/include/spice-streaming-agent/plugin.hpp
>> +++ b/include/spice-streaming-agent/plugin.hpp
>> @@ -26,7 +26,7 @@ class FrameCapture;
>>  * where MM is major and mm is the minor, can be easily expanded
>>  * using more bits in the future
>>  */
>> -enum Constants : unsigned { PluginVersion = 0x100u };
>> +enum Constants : unsigned { PluginVersion = 0x001u };
>> 
>> enum Ranks : unsigned {
>>     /// this plugin should not be used
>> diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
>> index 192054a..51b4504 100644
>> --- a/src/concrete-agent.cpp
>> +++ b/src/concrete-agent.cpp
>> @@ -34,6 +34,9 @@ ConcreteAgent::ConcreteAgent()
>> bool ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion) const
>> {
>>     unsigned version = Version();
>> +    // Accept API/ABI changes until we reached a stable version
>> +    if (MajorVersion(version) == 0)
>> +        return version == pluginVersion;
>>     return MajorVersion(version) == MajorVersion(pluginVersion) &&
>>         MinorVersion(version) >= MinorVersion(pluginVersion);
>> }

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]