Make HTB and HFSC Pretty, Named Consistently, and maybe Better Help

[EricLuehrsen]

• Renamed HTB/HFSC Simple/Simplest scripts and their help; so looks good in Luci
• Updated help to reflect recent discussions. Just so-so better, but more novice language still required
• Updated HFSC internal comments to reflect recent discussions
• Updated HTB internal comments and white space to be pretty, but no material change
• HFSC Simple (was Lite) is yet simpler, removing uncommon "home server" from isolated port tiers

[moeller0]

Hi Eric,

I appreciate action here, though I am not a fam of all of the changes:

On Feb 27, 2016, at 07:03 , Eric Luehrsen notifications@github.com wrote:

• Renamed HTB/HFSC Simple/Simplest scripts and their help; so looks good in Luci
• Updated help to reflect recent discussions. Just so-so better, but more novice language still required

I think we should keep the individual blurbs short and add a general policy help text that explains the differences between HTB and HFSC based shapers, so we do not need to repeat this 4 times. I think adding the tradeoff “sacrifices bandwidth to maintain the requested latency” and “sacrifices some latency to maintain the requested bandwidth” are worth repeating, but the explanation would better live in a more verbose help text IMHO.
Also I not sure whether the HTB and HFSC prefixes are really the ideal. These are implementation details while the real distinction that should be user-relevant is “latency-conserving” and “bandwidth-conserving” (not that these are good alternatives wordings either). What I mean is the names should allow a relative naive user to select the fitting script almost intuitively (this is what Dave tried with simple and simplest I believe).
Ah and this “This three band HTB configuration is intended for the delay control enthusiast.” is too cute to live, especially if compared with “This three band HFSC configuration is intended for common residential router as WAN gateway to cable or other broadband connection”. I know you like your own scripts best, but this does not even mention the only setup where the script proved superior in testing heavy traffic via WLAN… That said the ball is in my court to better document the DSCP mappings for htb_simple.qos and layer_cake.qos

To be clear and explicit I endorse your changes and are basically trying to discuss future changes to improve the documentation even more.

One thing I wondered, isn’t there a rename option in git? On github this looks like a full deletion and creation of new files, but that seems overly heavy handed, after all you only changed the filename…

Best Regards

• Updated HFSC internal comments to reflect recent discussions
• Updated HTB internal comments and white space to be pretty, but no material change
• HFSC Simple (was Lite) is yet simpler, removing uncommon "home server" from isolated port tiers
You can view, comment on, or merge this pull request online at:

#29

Commit Summary

• Rename hfsc_lite.qos to hfsc_simple.qos
• Rename hfsc_lite.qos.help to hfsc_simple.qos.help
• Rename hfsc_litest.qos to hfsc_simplest.qos
• Rename hfsc_litest.qos.help to hfsc_simplest.qos.help
• Rename simple.qos to htb_simple.qos
• Rename simple.qos.help to htb_simple.qos.help
• Rename simplest.qos to htb_simplest.qos
• Rename simplest.qos.help to htb_simplest.qos.help
• Merge pull request #1 from EricLuehrsen/pretty
• Update hfsc_simple.qos
• Update hfsc_simple.qos
• Update hfsc_simple.qos.help
• Update hfsc_simple.qos.help
• Update hfsc_simplest.qos.help
• Update htb_simple.qos.help
• Update htb_simplest.qos.help
• Update hfsc_simplest.qos
• Update htb_simple.qos
• Update htb_simple.qos
• Update htb_simplest.qos
• Update htb_simplest.qos
• Update htb_simplest.qos
• Update htb_simple.qos
• Update htb_simple.qos
• Update htb_simple.qos
• Merge pull request #2 from EricLuehrsen/pretty
File Changes

• D src/hfsc_lite.qos.help (1)
• D src/hfsc_litest.qos.help (1)
• R src/hfsc_simple.qos (109)
• A src/hfsc_simple.qos.help (1)
• R src/hfsc_simplest.qos (31)
• A src/hfsc_simplest.qos.help (1)
• A src/htb_simple.qos (307)
• A src/htb_simple.qos.help (1)
• A src/htb_simplest.qos (146)
• A src/htb_simplest.qos.help (1)
• D src/simple.qos (254)
• D src/simple.qos.help (1)
• D src/simplest.qos (110)
• D src/simplest.qos.help (1)
Patch Links:

• https://github.com/tohojo/sqm-scripts/pull/29.patch
• https://github.com/tohojo/sqm-scripts/pull/29.diff
—
Reply to this email directly or view it on GitHub.

[EricLuehrsen]

Yes, the first thing I did was just use "rename" and commit. The "diff" by text report actually shows it as a name change. I dont know why the log is A/D.

I didnt want to dig into presentation in Luci directly yet... Start with pretty scripts and help files first with tools that exist. Then shuffle common stuff upto a Luci intro paragraph or something. I hope we all update the for each help first. It will be wordy, yes. Then we can judge how to make a common help intro paragraph.

[moeller0]

On Feb 27, 2016, at 15:14 , Eric Luehrsen notifications@github.com wrote:

Yes, the first thing I did was just use "rename" and commit. The "diff" by text report actually shows it as a name change. I dont know why the log is A/D.

Sorry, I guess I should have looked at the actual commits instead of just at the github representation.

I didnt want to dig into presentation in Luci directly yet... Start with pretty scripts and help files first with tools that exist. Then shuffle common stuff upto a Luci intro paragraph or something.

Ah, I would/will just add a file to /usr/lib/sqm, namely Overview.qos.help the luci code is such that all files named *.qos.help will be displayed as text below the “Queue setup script” selection. So no luci fiddling required ;)

Best Regards

—
Reply to this email directly or view it on GitHub.

[EricLuehrsen]

That works. I would call it "an.overview.qos.help" so that comes first in the list.

[moeller0]

Hi Eric,

On Feb 27, 2016, at 19:41 , Eric Luehrsen notifications@github.com wrote:

That works. I would call it “an.overview.qos.help" so that comes first in the list.

Good idea, unfortunately we do not have easy control over sort order; in my case I installed hnyman’s build which includes sqm-scripts in the base image, all the .qos.help files in the root squashfs are listed before the later added files (in overtlay) so we will not be able to do this right for all installations. That said having a readonable ordering in the typical case might be more important that being correct in all cases ;)

Best Regards

—
Reply to this email directly or view it on GitHub.

[EricLuehrsen]

For us developers, that is a minor inconvenience. For the average user, the default install will look correct. I am noticing one thing with this implementation. The ".qos.help" interpreter will squash all paragraph breaks into one mash of words. So this is still only a crutch until we get something into Luci directly.

[tohojo]

While I appreciate what you're trying to do, I am not going to merge this pull request in its current form. For several reasons:

1. We can't just go changing the names of the scripts; this will break people's configuration if then update the sqm-scripts package. At the very least there needs to be backwards compatibility in the way this is done. However, I am thinking that since this is really a UI issue, the right way to go about this is to completely rework the way we display these things to the user, and maybe decouple it entirely from the script names.
2. The commits lack proper descriptive commit messages and signed-off-by tags. And several of them conflate different types of changes into one commit.
3. The history is a mess: There are way too many commits, and there are even merge commits in there. You need to rebase to get a clean history before applying your changes, then group commits into logical chunks. This also results in this pull request undoing previous changes; commit 56677e6 at least, maybe others.

The five bullet points in the pull request description are reasonable candidates for the logical commits; a fixed pull request with the latter three of those should be mergeable straight away. Let's hold off on the help text changes and renaming for now.

I'll go try to figure out a better way to display the help text...

[EricLuehrsen]

(1) (2) Acknowledged. Just had my own on going trials, and figured its worth the proposal. This is the feedback I was hoping for.

(3a) There are aspects of GitHub that I do not like. Historical minutia survive merges. My branch "adventuresintostupidity" with all incremental errors should be able to merge with roll up into my own "master."

(3b) I think I can reconstruct the pull on that.

(3c) I thought I rebased including 56677e6.

[tohojo]

Yeah, the github interface is really only suitable for the most basic of things. You really need to use the command line for anything more than that.

What I usually do when pushing things upstream somewhere is have a separate branch for experimenting and doing incremental work, then cloning that to another branch and using git rebase -i to turn that into nice logical commits with proper descriptions. That can then be pushed somewhere and used for pull requests...