Terminator

Merge lp://staging/~jtyr/terminator/terminator into lp://staging/terminator/trunk

terminator
Merge into trunk

Proposed by Jiri Tyr on 2014-03-06

Status:	Merged
Merged at revision:	1564
Proposed branch:	lp://staging/~jtyr/terminator/terminator
Merge into:	lp://staging/terminator/trunk
Diff against target:	164 lines (+85/-2) 5 files modified doc/terminator_config.5 (+4/-0) terminatorlib/config.py (+1/-0) terminatorlib/preferences.glade (+53/-0) terminatorlib/prefseditor.py (+22/-0) terminatorlib/terminator.py (+5/-2)
To merge this branch:	bzr merge lp://staging/~jtyr/terminator/terminator
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Stephen Boddy	2014-03-06	Approve on 2015-06-22
Jiri Tyr (community)		Needs Resubmitting on 2014-05-08
Review via email: mp+209719@code.staging.launchpad.net

Description of the change

I have noticed that the Broadcast is set to "group" by default. I believe that this is wrong and it should be set to "off" by default.

Revision history for this message

Stephen Boddy (stephen-j-boddy) wrote on 2014-03-07:

Hi Jiri, as the current maintainer and original contributor of that area of code I disagree that the current behaviour is wrong. The groups are there to facilitate flexible broadcast modes. If you don't need to broadcast there's not much (if any) need to use groups. By default new terminals do not get a new group, so there should be no negative use case.

It would take a convincing counter-argument to make me consider changing the current default behaviour. A patch making this configurable (including GUI and man pages changes too) is more likely to be accepted than a change in the default behaviour.

review: Disapprove

lp://staging/~jtyr/terminator/terminator updated on 2014-03-07

1495. By Jiri Tyr on 2014-03-07: Configurable broadcast group

Revision history for this message

Jiri Tyr (jtyr) wrote on 2014-03-07:

Thanks for your comments. I have implemented the configurable broadcast group in the revision 1495. Please let me know if it looks fine or if I need to change something.

Revision history for this message

Jiri Tyr (jtyr) wrote on 2014-04-07:

Please could you review this change again? It's been one month since the commit and still no comment.

review: Needs Resubmitting

Revision history for this message

Jiri Tyr (jtyr) wrote on 2014-05-08:

Please can I get any feedback on this?

review: Needs Resubmitting

Revision history for this message

Stephen Boddy (stephen-j-boddy) wrote on 2015-06-22:

After all this time I finally merged this. Two minor differences,
1) I changed it from broadcast_group to broadcast_defaut.
2) I removed the line that causes the current mode to change. It felt unexpected when it happened, and I prefer the principle of least surprise. Change is picked up on next run.

review: Approve