GNOME Bugzilla – Bug 764258
[PATCH] Split lightsoff.vala in two files.
Last modified: 2018-05-22 11:40:54 UTC
Here is a patch to move the application logic in one file and the window one in another. That helps working on following changes.
Created attachment 324836 [details] [review] Split lightsoff.vala in two files.
Review of attachment 324836 [details] [review]: I would name the file either lightsoff.vala or simply main.vala, rather than lightsoff-main.vala. Similarly, lightsoff-window.vala I would name simply window.vala. There's no value in having the prefix lightsoff in each name, IMO, and we don't do that for other games.
Created attachment 324905 [details] [review] Split lightsoff.vala in two files. Forgot the po/* files. (In reply to Michael Catanzaro from comment #2) > Review of attachment 324836 [details] [review] [review]: > I would name the file either lightsoff.vala or simply main.vala, rather than > lightsoff-main.vala. Quite sure “lightsoff.vala” is less clear than “lightsoff-main.vala”, but it’s probably not something that is important enough to take time to talk about. > Similarly, lightsoff-window.vala I would name simply window.vala. > There's no value in having the prefix lightsoff in each name, IMO, > and we don't do that for other games. We do it, and there’s a value in having the prefix lightsoff in each name, IMO. But we already discussed that elsewhere, and so that just continues to be time lost for interesting things.
Created attachment 324936 [details] [review] Move GSettings level management in game-view.vala. For now, the management of the game level is in the GameView class, but the update of the related GSettings key is in LightsoffWindow. It’s probably better to change that, so that the window doesn’t have to know anything.
Comment on attachment 324905 [details] [review] Split lightsoff.vala in two files. Committed this one, as per IRC talk. Keeping the other patch around and the bug open for now.
Review of attachment 324936 [details] [review]: ::: src/lightsoff-window.vala @@ +83,2 @@ { + int level = game_view.current_level; var
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/lightsoff/issues/5.