search

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

PR against bunsenlabs/deuterium #46

Closed xaosfiftytwo closed 8 years ago

xaosfiftytwo commented 8 years ago

3 commits.

xaosfiftytwo commented 8 years ago

@ John, I was in 2 minds about creating the PR against the master branch or deuterium. Should' ve asked first. No problem creating new PR against deuterium :)

xaosfiftytwo commented 8 years ago

Added new commit: Handle the absence of any config file. For when the user has removed both $HOME/.config/bl-exit/bl-exitrc and /etc/bl-exit/bl-exitrc. In that case, create an in memory configparser object with defaults. This makes sure that self.cp is never None and makes it slightly easier to determine if a button should be visible or not.

hhhorb commented 8 years ago

Just... great! Thanks for adding my suggestions!

johnraff commented 8 years ago

Has anyone tested this? If so we can merge.

xaosfiftytwo commented 8 years ago

New commit. Mostly cosmetic.

For the - rare case, I admit - that the user has a longish name and only 2 buttons are configured to be visible:

in stead of:

screenshot - 140716 - 13 19 11

She now gets:

screenshot - 140716 - 13 16 23

Users name is complete (up to about 20 characters) and its the users name that determines the window width. But the buttons are now SPREAD over the window width in stead of EDGED.

BTW: I have tested the complete latest version on a virgin Hydrogen install.

As far as I am concerned, the complete PR can be merged into master.

Of course more testing and remarks are welcomed.

johnraff commented 8 years ago

@xaosfiftytwo Posting new commits to an existing PR creates a moving target.

In future, I would like to suggest either waiting until all the commits in xaosfiftytwo:master are in place before sending the PR, or else waiting till the original PR has been merged before making the subsequent commits to a new one.

Anyway,

I have tested the complete latest version on a virgin Hydrogen install. As far as I am concerned, the complete PR can be merged into master.

OK I will merge this PR into the deuterium branch now.

xaosfiftytwo commented 8 years ago

@johnraff As you wish, John.

Though I mean the whole idea for a PR is to discuss, changing the PR until agreement is reached between the poster of the PR and the owner of the repo, and finalising with the owner merging the agreed upon last version of the PR.

johnraff commented 8 years ago

Well... your concept of the way to use PR's is certainly not ridiculous. I had somehow got the idea that they should be single-issue actions, just done and finished, and that suggestions and discussions should be done in "Issues".

Let's get some other opinions, and look around the web for guidance, before deciding policy on this.