Merge lp:~unity-api-team/connectivity-api/ci-testrun into lp:connectivity-api/14.10

Proposed by Antti Kaijanmäki
Status: Merged
Merged at revision: 9
Proposed branch: lp:~unity-api-team/connectivity-api/ci-testrun
Merge into: lp:connectivity-api/14.10
Diff against target: 140 lines (+14/-48)
6 files modified
CMakeLists.txt (+7/-4)
data/connectivity-cpp.pc.in (+1/-1)
debian/changelog (+1/-1)
debian/control (+4/-4)
debian/copyright (+1/-1)
src/platform/nmofono/link.h (+0/-37)
To merge this branch: bzr merge lp:~unity-api-team/connectivity-api/ci-testrun
Reviewer Review Type Date Requested Status
Didier Roche-Tolomelli Approve
PS Jenkins bot (community) continuous-integration Approve
Antti Kaijanmäki (community) Approve
Review via email: mp+210511@code.launchpad.net

Commit message

Changes based on packaging review.

Description of the change

Waiting for packaging review as NEW universe package.

This is a prerequisite for the new indicator-network used solely in unity8 and thus the catch-all unity8 FFe should apply.

To post a comment you must log in.
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

From ubuntu-core-dev the proposed branch needs a complete packaging review.

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

The purpose of this MP is to get through the initial ci train landing and to introduce a new source package to the universe.

The initial code is already in lp:connectivity-api and this MR contains no code changes. The only meaningful review comes from the ubuntu-core-dev team and thus I'm claiming the Unity API Team side of the review myself.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Review done by IRC: This change is fine itself, however, it will need some modification to enter the archive, so maybe use that branch?

09:07:47 didrocks | Wellark: as we try to standardize the packaging, can you move the "Section" stenza just after Priority please?
                  | (nitpick)
09:07:59 didrocks | in debian/control
09:09:05 didrocks | Wellark: you're forcing gcc 4.7 because of properties-cpp? :)
09:09:55 didrocks | Wellark: not sure why are you depending on libdbus-1-dev, shouldn't libdbus-cpp-dev brings it in? (and you only
                  | use the cpp bindings, right?)
09:10:59 didrocks | Wellark: same, to standardize, can you put recommends after depends?
09:11:15 didrocks | Wellark: also, on Package: libconnectivity-cpp0
09:11:25 didrocks | no need to repeat the section as it matches the source one
09:11:52 didrocks | Wellark: also, please suggest the -doc, not recommends
09:13:34 didrocks | Wellark: just a note once you will want to enter main: you will need a symbols file
09:13:46 didrocks | so bonus point if you add it now :)
09:14:42 didrocks | Wellark: your version should be 0.0.1-0ubuntu1 btw in debian/changelog
09:18:46 didrocks | Wellark: your .pc file Requires: dbus-cpp
09:19:02 didrocks | so Package: libconnectivity-cpp-dev
09:19:13 didrocks | shouldn't dep on libproperties-cpp-dev, but on libdbus-cpp-dev
09:19:22 didrocks | (bonus for trailing stenza)

Revision history for this message
Didier Roche-Tolomelli (didrocks) :
review: Needs Fixing
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> Review done by IRC: This change is fine itself, however, it will need some
> modification to enter the archive, so maybe use that branch?

Yep. let's fix it all here.

7. By Antti Kaijanmäki

debian/control: Section after Priority.

8. By Antti Kaijanmäki

Clarify why to force g++-4.7.

9. By Antti Kaijanmäki

Recommends after depends.

10. By Antti Kaijanmäki

Remove redundant Section.

11. By Antti Kaijanmäki

Suggests -dev instead of Recommends

12. By Antti Kaijanmäki

Fix debian/changelog version

13. By Antti Kaijanmäki

add -dev Depends on libdbus-cpp-dev

Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

> 09:07:47 didrocks | Wellark: as we try to standardize the packaging, can you
> move the "Section" stenza just after Priority please?
> | (nitpick)

Done.

> 09:07:59 didrocks | in debian/control
> 09:09:05 didrocks | Wellark: you're forcing gcc 4.7 because of properties-cpp?
> :)

Added a comment.

> 09:09:55 didrocks | Wellark: not sure why are you depending on libdbus-1-dev,
> shouldn't libdbus-cpp-dev brings it in? (and you only
> | use the cpp bindings, right?)

Known bug:
https://bugs.launchpad.net/ubuntu/+source/dbus-cpp/+bug/1272105

> 09:10:59 didrocks | Wellark: same, to standardize, can you put recommends
> after depends?

Done.

> 09:11:15 didrocks | Wellark: also, on Package: libconnectivity-cpp0
> 09:11:25 didrocks | no need to repeat the section as it matches the source one

Done.

> 09:11:52 didrocks | Wellark: also, please suggest the -doc, not recommends

Done.

> 09:13:34 didrocks | Wellark: just a note once you will want to enter main: you
> will need a symbols file
> 09:13:46 didrocks | so bonus point if you add it now :)

Noted. I don't want to deal with the pain right now.
The library will still get modifications and dealing with the symbols pain this early will just slow everything down to a halt.

> 09:14:42 didrocks | Wellark: your version should be 0.0.1-0ubuntu1 btw in
> debian/changelog

Fixed.

> 09:18:46 didrocks | Wellark: your .pc file Requires: dbus-cpp
> 09:19:02 didrocks | so Package: libconnectivity-cpp-dev
> 09:19:13 didrocks | shouldn't dep on libproperties-cpp-dev, but on libdbus-
> cpp-dev

Actually I need to depend on both. I need the dbus-cpp.pc to get the linker flags from pkg-config, although I'm not exposing any of the dbus-cpp on my public interface.

And libproperties-cpp-dev is needed as I'm using it on my public interface.

> 09:19:22 didrocks | (bonus for trailing stenza)

Don't know what that means..

14. By Antti Kaijanmäki

.pc: add properties-cpp to the Requires.

15. By Antti Kaijanmäki

Trailing comma on Depends.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Perfect, thanks a lot!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2014-03-11 16:07:46 +0000
3+++ CMakeLists.txt 2014-03-12 10:34:29 +0000
4@@ -18,6 +18,13 @@
5 cmake_minimum_required(VERSION 2.8)
6 project(connectivity-cpp)
7
8+# We are doing this because dbus-cpp and friends are stuck with 4.7 because of
9+# platform-api and there is an ABI break between 4.7 and 4.8.
10+# After we switch to 4.8 there is a mandatory soname bump.
11+message(STATUS "Remember, you need to compile this with GCC 4.7 or there will be bugs.")
12+message(STATUS "Package builds to this automatically. To do it manually do this:")
13+message(STATUS "CC=gcc-4.7 CXX=g++-4.7 cmake <your opts>")
14+
15 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules)
16
17 include(EnableCoverageReport)
18@@ -109,7 +116,3 @@
19
20 enable_coverage_report(TARGETS connectivity-cpp
21 FILTER ${CMAKE_SOURCE_DIR}/tests/* ${CMAKE_BINARY_DIR}/*)
22-
23-message(STATUS "Remember, you need to compile this with GCC 4.7 or there will be bugs.")
24-message(STATUS "Package builds to this automatically. To do it manually do this:")
25-message(STATUS "CC=gcc-4.7 CXX=g++-4.7 cmake <your opts>")
26
27=== modified file 'data/connectivity-cpp.pc.in'
28--- data/connectivity-cpp.pc.in 2014-02-05 19:32:11 +0000
29+++ data/connectivity-cpp.pc.in 2014-03-12 10:34:29 +0000
30@@ -8,4 +8,4 @@
31 Version: @CONNECTIVITY_CPP_VERSION_MAJOR@.@CONNECTIVITY_CPP_VERSION_MINOR@.@CONNECTIVITY_CPP_VERSION_PATCH@
32 Cflags: -I${includedir}/connectivity-cpp-@CONNECTIVITY_CPP_VERSION_MAJOR@
33 Libs: -L${libdir} -lconnectivity-cpp
34-Requires: dbus-cpp
35+Requires: dbus-cpp properties-cpp
36
37=== modified file 'debian/changelog'
38--- debian/changelog 2014-03-11 21:08:34 +0000
39+++ debian/changelog 2014-03-12 10:34:29 +0000
40@@ -1,4 +1,4 @@
41-connectivity-api (0.0.1) UNRELEASED; urgency=low
42+connectivity-api (0.0.1-0ubuntu1) UNRELEASED; urgency=low
43
44 * Initial release.
45
46
47=== modified file 'debian/control'
48--- debian/control 2014-03-11 21:08:34 +0000
49+++ debian/control 2014-03-12 10:34:29 +0000
50@@ -1,5 +1,6 @@
51 Source: connectivity-api
52 Priority: optional
53+Section: libs
54 Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
55 Build-Depends: cmake,
56 dbus,
57@@ -18,13 +19,11 @@
58 network-manager-dev,
59 pkg-config,
60 Standards-Version: 3.9.5
61-Section: libs
62 Homepage: https://launchpad.net/connectivity-api
63 Vcs-Bzr: lp:connectivity-api
64 Vcs-Browser: https://bazaar.launchpad.net/~unity-api-team/connectivity-api/trunk.14.04/files
65
66 Package: libconnectivity-cpp0
67-Section: libs
68 Architecture: any
69 Multi-Arch: same
70 Pre-Depends: ${misc:Pre-Depends}
71@@ -39,10 +38,11 @@
72 Section: libdevel
73 Architecture: any
74 Multi-Arch: same
75-Recommends: libconnectivity-cpp-doc
76 Depends: ${misc:Depends},
77 libconnectivity-cpp0 (= ${binary:Version}),
78- libproperties-cpp-dev
79+ libdbus-cpp-dev,
80+ libproperties-cpp-dev,
81+Suggests: libconnectivity-cpp-doc
82 Description: C++11 library providing connectivity API. - dev headers
83 All the development headers and libraries for libconnectivity-cpp.
84
85
86=== modified file 'debian/copyright'
87--- debian/copyright 2014-01-23 22:09:36 +0000
88+++ debian/copyright 2014-03-12 10:34:29 +0000
89@@ -15,7 +15,7 @@
90 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
91 Lesser General Public License for more details.
92 .
93- You should have received a copy of the GNU General Public License
94+ You should have received a copy of the GNU Lesser General Public License
95 along with this program. If not, see <http://www.gnu.org/licenses/>.
96 .
97 On Debian systems, the complete text of the GNU Lesser General
98
99=== removed file 'src/platform/nmofono/link.cpp'
100=== removed file 'src/platform/nmofono/link.h'
101--- src/platform/nmofono/link.h 2014-01-23 22:09:36 +0000
102+++ src/platform/nmofono/link.h 1970-01-01 00:00:00 +0000
103@@ -1,37 +0,0 @@
104-/*
105- * Copyright (C) 2013 Canonical, Ltd.
106- *
107- * This program is free software: you can redistribute it and/or modify it
108- * under the terms of the GNU General Public License version 3, as published
109- * by the Free Software Foundation.
110- *
111- * This program is distributed in the hope that it will be useful, but
112- * WITHOUT ANY WARRANTY; without even the implied warranties of
113- * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
114- * PURPOSE. See the GNU General Public License for more details.
115- *
116- * You should have received a copy of the GNU General Public License along
117- * with this program. If not, see <http://www.gnu.org/licenses/>.
118- *
119- * Authors:
120- * Antti Kaijanmäki <antti.kaijanmaki@canonical.com>
121- */
122-
123-#ifndef PLATFORM_TEST_LINK
124-#define PLATFORM_TEST_LINK
125-
126-#include <com/ubuntu/connectivity/networking/link.h>
127-
128-namespace platform {
129-namespace test {
130- class Link;
131-}
132-}
133-
134-class platform::test::Link : public com::ubuntu::connectivity::networking::Link
135-{
136-public:
137-
138-};
139-
140-#endif

Subscribers

People subscribed via source and target branches

to all changes: