Updated README.md
ClosedPublic

Authored by varunp on Nov 20 2018, 2:25 AM.

Details

Summary

Updated outdated README as discussed in a GCI task, including updating features list and adding a link to build instructions.

Test Plan

N/A

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
varunp created this revision.Nov 20 2018, 2:25 AM
Restricted Application added a project: KDE Connect. Β· View Herald TranscriptNov 20 2018, 2:25 AM
Restricted Application added a subscriber: kdeconnect. Β· View Herald Transcript
varunp requested review of this revision.Nov 20 2018, 2:25 AM
varunp retitled this revision from # Enter a commit message. # # Changes: # # README.md Updated README.md with some new features (slideshow control) and added link to build instructions. to Updated README.md.Nov 20 2018, 2:26 AM
varunp added a reviewer: albertvaka.
albertvaka requested changes to this revision.Nov 20 2018, 9:27 AM

Found some typos and a small change.

README.md
8

Typos: filesystem integration.

26

Windows mostly works. We can say something like: "There hasn't been an official Windows release yet, but many features have been ported to Windows. You can build it yourself using Craft."

This revision now requires changes to proceed.Nov 20 2018, 9:27 AM

This is good. Thank you!
I have added a few comments for making the English sound a little more natural, as well as some little fixes

README.md
13

"already existing" is redundant -- I would just change it to "over your WiFi network" or even just "over WiFi"

16–17

I don't know if we need to add a comment about distro support, but I would change the addition to something like "Any disto which supports Qt 5"
Better to be specific about what "little work" is needed :)

20

These typos were already here:
"This explains how to install KDE Connect on your computer. You will also need to install it in on your Andriod device and pair it to your computer in the app if you want it to be any useful."

But I would actually reword this to:
"These instructions explain how to install KDE Connect on your computer. You will also need to install it on your Andriod device and pair both devices before you can use this app"

(I prefer "pair both devices" vs. "pair it to your computer" because it avoids mirroring "your computer" in both sentences. It sounds more natural to me)

29

I would leave this sentence as it originally was. "might" means something like "Oh, well, we don't really know" but "should" means "We expect that it will work even though we don't officially support it" (And I think it does work?)

59

Don't you have to specify that we are channel #kdeconnect on freenode? I know almost nothing about IRC πŸ˜‚

varunp updated this revision to Diff 46100.Nov 24 2018, 5:31 AM
This comment was removed by varunp.

My changes didn't seem to commit properly, ill try this again...

varunp updated this revision to Diff 46102.Nov 24 2018, 5:50 AM
  • Updated to reflect reviews.
varunp marked 5 inline comments as done.Nov 24 2018, 6:06 AM

Fixed most of it, missed one error, will fix that now.

README.md
29

Maybe make testing this a GCI task?

59

Yes we do, good catch.

varunp updated this revision to Diff 46103.Nov 24 2018, 6:10 AM
varunp marked 2 inline comments as done.
  • updated 'to' to 'on' your android device
varunp marked an inline comment as done.Nov 24 2018, 6:32 AM

fixed

I'm happy. I'll leave it for Albert to click the final approval :)

albertvaka accepted this revision.Nov 26 2018, 11:36 AM

I found something else we could change, but all the changes look good to me. Can you update the diff and I will merge it? See inline.

README.md
23

I don't think this paragraf is relevant anymore: we already provide an appindicator and Nautilus integration in our repo. Also probably GSConnect is more mature than indicator-kdeconnect nowadays. Can you remove it?

This revision is now accepted and ready to land.Nov 26 2018, 11:36 AM
varunp updated this revision to Diff 46293.Nov 27 2018, 12:17 AM
  • updated 'to' to 'on' your android device
varunp marked an inline comment as done.Nov 27 2018, 12:21 AM

Paragraph has been removed, you can commit now.

Closed by commit R224:a7db44aca21d: Updated README.md (authored by varunp, committed by apol). Β· Explain WhyNov 28 2018, 1:38 PM
This revision was automatically updated to reflect the committed changes.