Page MenuHomeSolus

Initial inclusion of gestures
ClosedPublic

Authored by cunidev on Jun 10 2018, 12:11 PM.

Details

Summary

Resolves T6334

Test Plan

Author of commit is developer of software. Testing is implied.

Diff Detail

Repository
R4326 gestures
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cunidev created this revision.Jun 10 2018, 12:11 PM
cunidev requested review of this revision.Jun 10 2018, 12:11 PM
JoshStrobl abandoned this revision.Jun 13 2018, 3:33 PM
JoshStrobl added a subscriber: JoshStrobl.
  1. You shouldn't be including the eopkg.
  2. Makefile should be -rw-r--r--
  3. Check other comments in patch and feel free to re-open or create a new diff when build system remarks have been addressed.
package.yml
17

Except you don't need the exact path of an executable in a desktop file, so not sure why /usr/local/bin is in there in the first place, you just need the binary somewhere exposed in your PATH.

18–27

Honestly, this is really jank. I can't really accept it in this sort of state, with a fairly flawed "build system". I'd suggest looking into using Python's setuptools, given you've written it in python.

cunidev updated this revision to Diff 7658.Jun 14 2018, 3:51 PM

Corrected and implemented setup.py

@JoshStrobl apologies for the duplicate. Is everything fine now?

Once again, please ensure you are only doing a single commit instead of multiple (can tell by the change between the initial and the changes you just pushed). Instructions are provided in the documentation I linked in your other diff, you may need to squash your commits (which there are instructions for in the doc as well).

cunidev updated this revision to Diff 7663.Jun 14 2018, 4:17 PM

implemented setup.py

Well, I wouln't call my first attempt at packaging exactly successful :/ anything else to fix? @JoshStrobl

JoshStrobl requested changes to this revision.Jun 14 2018, 4:27 PM
JoshStrobl added inline comments.
package.yml
8

Should probably be system.utils

12

Remove newline.

15

Remove newline.

19

So is this written in python 2 or 3? If python3, should be using python3-gobject instead, and remove python3 as a rundep.

23

Should be using 4 spaces.

This revision now requires changes to proceed.Jun 14 2018, 4:27 PM
cunidev added a comment.EditedJun 14 2018, 5:37 PM

@JoshStrobl sure about system.utils? tried to use the same category as libinput, the tool the gui's based on
edit: easystroke, another gesture-related app, is also listed as "desktop"

cunidev updated this revision to Diff 7667.Jun 14 2018, 5:47 PM

package.yml fixes

JoshStrobl accepted this revision.Jun 21 2018, 5:39 PM

LGTM, thanks for your hard work!

This revision is now accepted and ready to land.Jun 21 2018, 5:39 PM
JoshStrobl retitled this revision from gestures first commit (T6334, 0.1.4) to Initial inclusion of gestures.Jun 21 2018, 5:40 PM
JoshStrobl edited the summary of this revision. (Show Details)
JoshStrobl edited the test plan for this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.

Thanks for yours :)