BunsenLabs / bunsen-utilities

https://pkg.bunsenlabs.org/debian/pool/main/b/bunsen-utilities/
GNU General Public License v3.0
31 stars 21 forks source link

bl-exit: Make processing from commandline work again. #38

Closed xaosfiftytwo closed 8 years ago

xaosfiftytwo commented 8 years ago

Please note that debian/changelog has already been changed for version 8.6.6-1. I have removed tag 8.6.6-1 so that it can be recreated when this pull request is accepted.

Hjdskes commented 8 years ago

I'll comment on this tomorrow, just so you know :)

johnraff commented 8 years ago

Please note that debian/control has already been changed for version 8.6.6-1.

Not actually so: debian/control contains no version information, and was last modified 2 months ago.

xaosfiftytwo commented 8 years ago

Hi John, Welcome back :)

I wrote debian/control, but I meant debian/changelog Changed original comment now.

Hjdskes commented 8 years ago

Sorry for being late. Will look at it now.

Hjdskes commented 8 years ago

In my opinion there is too much duplicated code currently (not just in this PR).

Examples:

        try:
            do_it("PowerOff")
        except (subprocess.CalledProcessError, CanDoItError, KeyError) as e:
            emDialog = gtk.MessageDialog(parent=None, flags=0, type=gtk.MESSAGE_INFO,
                                         buttons=gtk.BUTTONS_OK, message_format=None)
            if emDialog:
                emDialog.set_markup("{}".format(e))
                emDialog.run()
        finally:
            gtk.main_quit()

is repeated several times and all that differs is the argument to do_it.

Another example is

subprocess.check_output(["dbus-send", "--print-reply", "--system",
                                        "--dest=org.freedesktop.login1",
                                        "/org/freedesktop/login1",
                                        "org.freedesktop.login1.Manager.{}".format(actionMethod) ])

and the send_dbus method. These are not exactly alike, but perhaps ^ should be made into its own function as well.

Then there is

    try:
        action="PowerOff"
        do_it(action)
    except (subprocess.CalledProcessError, CanDoItError, KeyError) as e:
        print("{}".format(e), file=sys.stderr)
        sys.exit(1)

which is repeated four times, again only action= being different.

xaosfiftytwo commented 8 years ago

OK. Seems to make sense. Will work on it.

johnraff commented 8 years ago

BTW just to throw in my 2c: I think it looks more professional to show a "hibernate" button only if it's likely to work, ie after it's passed the tests. (Let any error messages go to STDERR, where they'll end up in ~/.xsession-errors.)

Many users don't need hibernate anyway, but if someone clicked the button on a system where it wasn't supported, only to get an error popup, then they might feel they'd been tricked.

Users who really want hibernate can ask for support on the forums on how to make the button appear.

xaosfiftytwo commented 8 years ago

Seems like the 'Hibernate' feature still needs some discussion. Do we discuss it here? Involve the end users in it?

I am not trying to force anything on anyone. So, I suggest we reset the github repo to the situation of tag 8.6.5-1 to discard my changes after that, then first re-enable the accelerations, and hold off any hibernation related changes until we can agree on what we will implement - if anything.

Can someone more experienced with github than me do the reset?

hhhorb commented 8 years ago

Seems like the 'Hibernate' feature still needs some discussion. Do we discuss it here? Involve the end users in it?

Start a forum thread, please.

johnraff commented 8 years ago

So, I suggest we reset the github repo to the situation of tag 8.6.5-1 to discard my changes after that,

I've just had to revert changes on bunsen-pipemenus and bunsen-welcome because I pushed changes to "master" instead of the new branch "newpaths" . The command that did it for me was git revert <shasum> to go back to the previous commit. To go back several commits it looks as if git revert <shasum>..HEAD might do it. (There are multiple methods posted on the web of course: checkout + commit looks like another feasable way.)

Shall I give it a shot, or would @2ion or @Unia like to do it?

Hjdskes commented 8 years ago

Reverting one or several commits indeed works as you described John, however perhaps you want to add --no-commit when executing:

Usually the command automatically creates some commits with commit log messages stating which commits were reverted. This flag applies the changes necessary to revert the named commits to your working tree and the index, but does not make the commits. In addition, when this option is used, your index does not have to match the HEAD commit. The revert is done against the beginning state of your index.

This is useful when reverting more than one commits' effect to your index in a row.

ghost commented 8 years ago

On a given branch, before it's merged, just go

git reset --hard $COMMIT

to discard all history after $COMMIT. $COMMIT is the hash you get from git log or similar.

Hjdskes commented 8 years ago

Reverting is safer, especially once the commits have been pushed to a public branch as in this case.

ghost commented 8 years ago
push --force

]:D But you have a point.

johnraff commented 8 years ago

In this case there were many commits to revert, including several merges, which complicate the situation because you have to specify which pre-merge fork is the one you're following.

Eventually I found a different approach - make a temporary branch from tag, reset --soft master to put the HEAD up with the master branch, and commit the difference to get back to the state of tag. Then the new branch can be merged back into master.

ie, this seemed to work on my local repo, so I've pushed it up to GitHub. I hope there are no issues:

git branch goback 8.6.5-1
git checkout goback # these two could have been combined: git checkout -b goback 8.6.5-1
git reset --soft master
git commit -m 'Undo all commits after tag 8.6.5-1
git checkout master
git merge goback
git push
git branch -d goback