If baloo is not running, monitor will wait for its start.
This will simplify baloo diagnostics.
Details
Details
- Reviewers
dfaure alexeymin - Commits
- R293:94da1850ec6c: balooctl monitor: Wait for dbus interface
$ balooctl stop $ balooctl monitor
Without patch:
Baloo is not running Press ctrl+c to exit monitor
With patch:
Press ctrl+c to exit monitor Waiting for Baloo to start
In different console: balooctl start
Without patch: Nothing happens
With patch:
Baloo is running QDBusConnection: name 'org.kde.baloo' had owner '' but we thought it was ':1.238'
Change a file in indexed folder. E.g.
$ echo otto >> ~/Documents/otto.csvWithout patch: nothing happens.
With patch:
Indexing: ~/Documents/otto.csv: Ok`
In different console: balooctl stop
With and without patch:
Baloo died $
Diff Detail
Diff Detail
- Repository
- R293 Baloo
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Can you add simple instructions how to reproduce full testing scenario?
| src/tools/balooctl/monitorcommand.cpp | ||
|---|---|---|
| 76 | No need to call instance() here; QCoreApplication::exit() is a static function and does not need object instance. | |
Comment Actions
- balooctl monitor: Use QCoreApplication::exit()
- balooctl monitor: Explicit lamba captures
Comment Actions
Looks almost fine, just fix these:
| src/tools/balooctl/monitorcommand.cpp | ||
|---|---|---|
| 45 | Strange formatting of commas in above 3 lines. It looks OK in constructor, where member initialization may be added later, but here, when the parameter count is fixed, no need to start new line with a comma. I'd suggest to keep as it was before | |
| 54 | delete m_dbusInterface; after this line, because below you are creating a new object? Each every 50 ms? Memory leaks here | |
| 58 | Again, strange formatting of commas | |
| 65 | Strange formatting of commas | |
| 67 | Strange formatting of commas | |
| 73 | Strange formatting of commas | |
| src/tools/balooctl/monitorcommand.h | ||
| 28 | Why dod you move local includes up? Usually they are below globals | |
| 60 | extra empty string? | |
Comment Actions
- balooctl monitor: Fix memory leak
- balooctl monitor: Whitespace cleanup
| src/tools/balooctl/monitorcommand.cpp | ||
|---|---|---|
| 45 | You're right. I just looked at the example in Qt's coding style rules. In fact it is operators BOL, commas EOL. | |
| 54 | Yeah, massively. Thanks for pointing that out. | |
| src/tools/balooctl/monitorcommand.h | ||
| 28 | Not in baloo's code it seems. So it's for consistency. | |
Comment Actions
There is also https://techbase.kde.org/Policies/Frameworks_Coding_Style. Guess I was wrong about #includes order, but honestly I've never seen commas at the start of the line.
make test passes, works as described. You can wait for other reviews of course...
Comment Actions
... and it refers to Qt.
I've never seen commas at the start of the line.
It's part of the npm coding-style. I found it strange too at first. Probably it makes more sense in javascript. Again i don't insist on (... thanks for your accepting this diff ...) it.
| src/tools/balooctl/monitorcommand.cpp | ||
|---|---|---|
| 45 | I have only seen commas at the beginning of a line for constructor initializers, because adding a new one would affect two lines, instead of inserting a single new line. It just looks better in diffs. Otherwise, yes, commas at the end, please. | |
| src/tools/balooctl/monitorcommand.cpp | ||
|---|---|---|
| 53 | Hmm, why not just use QDbusServiceWatcher's signals, to connect to serviceRegistered, if the service isn't available when we start? | |
| src/tools/balooctl/monitorcommand.cpp | ||
|---|---|---|
| 53 | The idea to do it that way came to late. :-) | |