Clean up existing documentation
ClosedPublic

Authored by bruns on Apr 8 2018, 1:39 PM.

Details

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.
michaelh created this revision.Apr 8 2018, 1:39 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptApr 8 2018, 1:39 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
michaelh requested review of this revision.Apr 8 2018, 1:39 PM
bruns added a subscriber: bruns.Apr 8 2018, 3:35 PM
bruns added inline comments.
README.md
5–7

Try to stick to 78 chars per line, makes it easier to read

25
michaelh updated this revision to Diff 31688.Apr 8 2018, 4:09 PM
  • Apply suggested changes
bruns added inline comments.Apr 8 2018, 4:38 PM
README.md
28

The icon/badge is
https://build.kde.org/buildStatus/icon?job=Frameworks%20baloo%20kf5-qt5%20SUSEQt5.9

Oh, and keep it at the top of the section

michaelh updated this revision to Diff 31739.Apr 9 2018, 1:46 PM
  • Apply suggested changes
  • Add KFileMetaData link
  • Correct new bug links
  • Change recommendation text
michaelh marked 2 inline comments as done.Apr 9 2018, 1:53 PM
michaelh added a subscriber: ngraham.
michaelh added inline comments.
README.md
36

@ngraham
Original text was "We recommend not packing Baloo for Windows..."

  • ? Keep original ?
  • ? Keep this ?
  • ? "We recommend not to ..." ?
bruns added inline comments.Apr 9 2018, 8:04 PM
README.md
31

While it may run on other unix based systems

", it is hardly tested. It is recommended to thoroughly test it on other systems, as it may affect system stability."

36

AFAIK, on windows only the library is built, but as a stub.

See:
R293:49fecea8e218ce03b01073c1604cfc0b683b1cc5

michaelh added inline comments.Apr 9 2018, 8:21 PM
README.md
36

If I understood R293:49fecea8e218ce03b01073c1604cfc0b683b1cc5 correctly baloo is a stub for every system except linux.
In that case the whole passage can be dropped. From "While it may run on other..." to "...than Baloo ever will."

michaelh added inline comments.Apr 9 2018, 8:57 PM
README.md
36

For FreeBSD everything is built, test are run in CI and pass. For Windows it's a stub: Only KF5Baloo.dll and KF5BalooEngine.dll. OSX?

"Baloo is developed and tested exclusively for Linux and FreeBSD. While it may run on other ​unix based systems. It is not tested and therefore not recommended." ?
Drop "We do not recommend to package..." ?

michaelh added inline comments.Apr 10 2018, 7:20 AM
README.md
36

"Baloo is developed and tested exclusively for 64-bit Linux and FreeBSD. It may run on other ​unix based systems. On those it should be thoroughly tested as Baloo may affect system stability.

Baloo may run on 32-bit systems, but also may not work correctly. Please..."

(Is there a better way to collaborate on texts? This is tedious and not very effective. @ngraham?)

Sorry, not aware of a better workflow for now, but yeah, this does not seem ideal. Perhaps real-time chat would be better, during a time when everyone's available.

README.md
36

I think the original paragraph was fine, but needed a bit of cleanup and grammar work. How's this?

"We recommend not packaging Baloo for Windows or macOS, as both these operating systems offer their own file searching solutions that integrate better than Baloo can."

michaelh added inline comments.Apr 10 2018, 6:59 PM
README.md
36
  • In CI Baloo is fully built and tested for Linux and FreeBSD.
  • In CI an untested stub for Windows is build. I think that is to fulfill the dependencies.
  • macOS is not mentioned anywhere (and I don't know anything about it).

If Baloo has to be present for the dependency, it makes no sense to recommend not packaging it. That, or I got it wrong.

bruns added inline comments.Apr 11 2018, 11:11 PM
README.md
36

Thats my understanding of the code as well.

bruns added a comment.Nov 2 2018, 12:25 AM

I think these changes are a sufficient improvement to push it as is, we can do more fixups later.

@ngraham - your opinion?

Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptNov 2 2018, 12:25 AM
ngraham accepted this revision.Nov 2 2018, 2:27 AM

Yeah, go for it. Such a shame that Michael disappeared.

This revision is now accepted and ready to land.Nov 2 2018, 2:27 AM
bruns commandeered this revision.Nov 3 2018, 2:54 PM
bruns added a reviewer: michaelh.
This revision was automatically updated to reflect the committed changes.