Code improvements for Gnuton PR.

[Martinski4GitHub]

Some code improvements & fine-tuning for the newly merged code that supports Gnuton F/W updates.

[ExtremeFiretop]

@Martinski4GitHub

This is all fantastic! Great great stuff! I'll start my review right away! Thank you again!

[Martinski4GitHub]

@Martinski4GitHub

This is all fantastic! Great great stuff! I'll start my review right away! Thank you again!

As you said, we had to "bite the bullet" at some point and merge the Gnuton PR into the development branch. I'll check back in the evening. Have a good Monday, bud!!

[ExtremeFiretop]

I just realized we opened over a hundred PRs between this one and the original Gnuton PR hahaha

[ExtremeFiretop]

@Martinski4GitHub

This is so clean. So nice. So pretty to look at.

I noticed a lot of it was just clarifying the logic already there, I'm curious, does that calm the warning in the linter tool? Or do they continue to exist?

Would it help if I just generally followed more closely to these guidelines? (Even though the logic is more or less the same?)

Also. I noticed there are times you expanded the if this then that condition, other times you shrink/contract the statements. Can you help me understand when which is best suited for what?

Feel free to flag some examples in the code (for study and learning on my part)

[ExtremeFiretop]

Also does this mean we are now at the regression testing stage?

[ExtremeFiretop]

@Martinski4GitHub This is all fantastic! Great great stuff! I'll start my review right away! Thank you again!

As you said, we had to "bite the bullet" at some point and merge the Gnuton PR into the development branch. I'll check back in the evening. Have a good Monday, bud!!

Yes you have no idea how happy I am to finally only have the one version open on my screen and not 2...

Constantly thinking.. okay so what's the equivalent for Gnuton? Sometimes it's identical, sometimes it's not 1 for 1 obviously as you know.

One version to rule them all! Most of the differences have been identified over this period though and the intersecting points have been addressed

[Martinski4GitHub]

I just realized we opened over a hundred PRs between this one and the original Gnuton PR hahaha

You're right; I had not noticed that, LOL!! The PR was opened a little over 4 months ago!!

[Martinski4GitHub]

Also does this mean we are now at the regression testing stage?

Yes, absolutely. Regression testing can begin anytime now.

[Martinski4GitHub]

@Martinski4GitHub

This is so clean. So nice. So pretty to look at.

I noticed a lot of it was just clarifying the logic already there, I'm curious, does that calm the warning in the linter tool? Or do they continue to exist?

Would it help if I just generally followed more closely to these guidelines? (Even though the logic is more or less the same?)

All "unquoted" variable assignment statements are flagged as warnings by the Linter tool due to non-compliance with coding best practices. Fixing those lines removed about 18 or so warnings so it’s best to follow the rule unless there’s a very specific reason not to, in which case the warning would be ignored. So the vast majority of warnings were in this category.

For example, these are "unquoted" variable assignments:

readonly isGNUtonFW=$(_GetFirmwareVariantFromRouter_)
...
...
local release_data=$(curl -s "$url")

And these are the same variable assignments but properly "double quoted" now:

readonly isGNUtonFW="$(_GetFirmwareVariantFromRouter_)"
...
...
local release_data="$(curl -s "$url")"

Other code was modified due to non-compliance with coding standards & best practices. I'll add the explanation as a code review comment so I can point out the affected code.

[ExtremeFiretop]

Thanks for all the examples! I'll take note of all these and make sure to try to remember that to help keep the linter tool cleaner of warnings :)

[Martinski4GitHub]

Also. I noticed there are times you expanded the if this then that condition, other times you shrink/contract the statements. Can you help me understand when which is best suited for what?

Feel free to flag some examples in the code (for study and learning on my part)

WRT the "expansion/contraction" of if-then statements, they’re not due to the Linter tool. It’s a stylistic change done primarily for readability & review purposes (i.e. making the code easier to read for the reviewers). The longer and more elaborate if-then clauses are almost always "expanded" for readability.

For example, the following if-then code is not as easy & quick to read & review:

if "$isGNUtonFW"; then printf "\nPlease copy your firmware update file (.w or .pkgtb) using the *original* ZIP filename to this directory:"
else printf "\nPlease copy your firmware ZIP file (using the *original* ZIP filename) to this directory:"; fi

When compared with the modified "expanded" version:

if "$isGNUtonFW"
then
    printf "\nPlease copy your firmware update file (.w or .pkgtb) using the *original* filename to this directory:"
else
    printf "\nPlease copy your firmware ZIP file (using the *original* ZIP filename) to this directory:"
fi

OTOH, when the if-then clauses are short & simple, the lines can simplified by "shrinking" them. For example, here's the BEFORE:

if "$isGNUtonFW"
then
    _ManageChangelogGnuton_ "view"
else
    _ManageChangelogMerlin_ "view"
fi

Here's the AFTER:

if "$isGNUtonFW"
then _ManageChangelogGnuton_ "view"
else _ManageChangelogMerlin_ "view"
fi

Note that there are no strict rules or guidelines to follow for this type of change; it’s a subjective call for what makes a piece of code much clearer for readability & review purposes as well as for future maintainability.

[ExtremeFiretop]

Also. I noticed there are times you expanded the if this then that condition, other times you shrink/contract the statements. Can you help me understand when which is best suited for what? Feel free to flag some examples in the code (for study and learning on my part)

WRT the "expansion/contraction" of if-then statements, they’re not due to the Linter tool. It’s a stylistic change done primarily for readability & review purposes (i.e. making the code easier to read for the reviewers). The longer and more elaborate if-then clauses are almost always "expanded" for readability.

For example, the following if-then code is not as easy & quick to read & review:

if "$isGNUtonFW"; then printf "\nPlease copy your firmware update file (.w or .pkgtb) using the *original* ZIP filename to this directory:"
else printf "\nPlease copy your firmware ZIP file (using the *original* ZIP filename) to this directory:"; fi

When compared with the modified "expanded" version:

if "$isGNUtonFW"
then
    printf "\nPlease copy your firmware update file (.w or .pkgtb) using the *original* filename to this directory:"
else
    printf "\nPlease copy your firmware ZIP file (using the *original* ZIP filename) to this directory:"
fi

OTOH, when the if-then clauses are short & simple, the lines can simplified by "shrinking" them. For example, here's the BEFORE:

if "$isGNUtonFW"
then
    _ManageChangelogGnuton_ "view"
else
    _ManageChangelogMerlin_ "view"
fi

Here's the AFTER:

if "$isGNUtonFW"
then _ManageChangelogGnuton_ "view"
else _ManageChangelogMerlin_ "view"
fi

Note that there are no strict rules or guidelines to follow for this type of change; it’s a subjective call for what makes a piece of code much clearer for readability & review purposes as well as for future maintainability.

I always figured this was a subjective change, but I wanted to get a sense as to what constitutes expanding it vs contracting it. (In your mind) So I can follow long.

So know I'll know that if it's "simple" i.e one simply task in the if/else I can contract. But if it's multiple longer strings then I should probably be expanding

[ExtremeFiretop]

Sorry for the delay in responses.

I told my girlfriend MerlinAU was ready for regression testing after some 4 months of work.

She asked how long that would take. I said "idk maybe days until we are confident" and she gave me a sad face and asked me to play a few rounds of a video game I bought (overcooked) with her in co-op mode on the couch so I figured it was fair if I was going to be busy for a bit 🤣

[Martinski4GitHub]

Note that there are no strict rules or guidelines to follow for this type of change; it’s a subjective call for what makes a piece of code much clearer for readability & review purposes as well as for future maintainability.

I always figured this was a subjective change, but I wanted to get a sense as to what constitutes expanding it vs contracting it. (In your mind) So I can follow long.

So know I'll know that if it's "simple" i.e one simply task in the if/else I can contract. But if it's multiple longer strings then I should probably be expanding

Yes, that's roughly "the rule of thumb" to keep in mind for readability & review purposes.

[ExtremeFiretop]

Speaking of coop on the couch, anymore thought on Plex? 😉 I'm now well over 1000 movies and TV shows. Currently watching some Americas Got Talent cause it's been a few years hahaha

[Martinski4GitHub]

Sorry for the delay in responses.

I told my girlfriend MerlinAU was ready for regression testing after some 4 months of work.

She asked how long that would take. I said "idk maybe days until we are confident" and she gave me a sad face and asked me to play a few rounds of a video game I bought (overcooked) with her in co-op mode on the couch so I figured it was fair if I was going to be busy for a bit 🤣

I hear you, bud!! And I fully understand the situation. I've gone through that myself many times over the years when I had to work overtime to meet a deadline or came home really late & tired from a long day at work, and my wife ended up eating dinner or watching a movie by herself. I know she understands the responsibilities & commitments I have at work, but OTOH she doesn't always have to like it. I get that, so whenever she announces: "Tonight is going to be movie night." I always tried to be there not only for her but also because I really enjoy watching movies next to her (sometimes she makes funny comments or reactions so it's more enjoyable :>).

Also, many times I've gone "radio silent" for a while here on GitHub because I get "summoned by my higher power." So I'm sure you know what I mean :>).

Anyway, no worries about any delay in replying. I always figure that something has come up that's more important than this hobby project of ours.

[Martinski4GitHub]

Speaking of coop on the couch, anymore thought on Plex? 😉 I'm now well over 1000 movies and TV shows. Currently watching some Americas Got Talent cause it's been a few years hahaha

I'll ask my wife again about checking out your list of movies available. She gets busy with her work and other personal stuff to take care of as well, so that's likely not at the top of her to-do list. But I'll remind her.

[ExtremeFiretop]

Speaking of coop on the couch, anymore thought on Plex? 😉 I'm now well over 1000 movies and TV shows. Currently watching some Americas Got Talent cause it's been a few years hahaha

I'll ask my wife again about checking out your list of movies available. She gets busy with her work and other personal stuff to take care of as well, so that's likely not at the top of her to-do list. But I'll remind her.

Sounds good! No rush, I just figured I'd mention the offer is still open. List of movies is only growing!

I think I'll start testing MerlinAU tomorrow evening, it's starting to get late and I don't want to rush anything. I want to take my time testing any features or flows we may have touched.

I still have to compile a list of my intended tests, but that will be some brain food after a coffee tomorrow morning 🤣

[Martinski4GitHub]

Speaking of coop on the couch, anymore thought on Plex? 😉 I'm now well over 1000 movies and TV shows. Currently watching some Americas Got Talent cause it's been a few years hahaha

I'll ask my wife again about checking out your list of movies available. She gets busy with her work and other personal stuff to take care of as well, so that's likely not at the top of her to-do list. But I'll remind her.

Sounds good! No rush, I just figured I'd mention the offer is still open. List of movies is only growing!

I think I'll start testing MerlinAU tomorrow evening, it's starting to get late and I don't want to rush anything. I want to take my time testing any features or flows we may have touched.

I still have to compile a list of my intended tests, but that will be some brain food after a coffee tomorrow morning 🤣

Sounds good. I've already started testing the code before I submitted the PR changes to make sure that nothing was obviously broken when doing an RMerlin F/W update. But I haven't gone and tested all the menu options. I might have time tomorrow evening (depending on the workload as the release deadline is near - so far my modules & components are testing well, but I get called to help investigate bugs in other modules to help out).

[ExtremeFiretop]

Speaking of coop on the couch, anymore thought on Plex? 😉 I'm now well over 1000 movies and TV shows. Currently watching some Americas Got Talent cause it's been a few years hahaha

I'll ask my wife again about checking out your list of movies available. She gets busy with her work and other personal stuff to take care of as well, so that's likely not at the top of her to-do list. But I'll remind her.

Sounds good! No rush, I just figured I'd mention the offer is still open. List of movies is only growing! I think I'll start testing MerlinAU tomorrow evening, it's starting to get late and I don't want to rush anything. I want to take my time testing any features or flows we may have touched. I still have to compile a list of my intended tests, but that will be some brain food after a coffee tomorrow morning 🤣

Sounds good. I've already started testing the code before I submitted the PR changes to make sure that nothing was obviously broken when doing an RMerlin F/W update. But I haven't gone and tested all the menu options. I might have time tomorrow evening (depending on the workload as the release deadline is near - so far my modules & components are testing well, but I get called to help investigate bugs in other modules to help out).

Sent you a PM on the forums with a link to my test plan. Have a goodnight buddy :)

[Martinski4GitHub]

Speaking of coop on the couch, anymore thought on Plex? 😉 I'm now well over 1000 movies and TV shows. Currently watching some Americas Got Talent cause it's been a few years hahaha

I'll ask my wife again about checking out your list of movies available. She gets busy with her work and other personal stuff to take care of as well, so that's likely not at the top of her to-do list. But I'll remind her.

Sounds good! No rush, I just figured I'd mention the offer is still open. List of movies is only growing! I think I'll start testing MerlinAU tomorrow evening, it's starting to get late and I don't want to rush anything. I want to take my time testing any features or flows we may have touched. I still have to compile a list of my intended tests, but that will be some brain food after a coffee tomorrow morning 🤣

Sounds good. I've already started testing the code before I submitted the PR changes to make sure that nothing was obviously broken when doing an RMerlin F/W update. But I haven't gone and tested all the menu options. I might have time tomorrow evening (depending on the workload as the release deadline is near - so far my modules & components are testing well, but I get called to help investigate bugs in other modules to help out).

Sent you a PM on the forums with a link to my test plan. Have a goodnight buddy :)

Looks very detailed & thorough. I like it!! I'll use it as a guide when testing RMerlin F/W updates. Have a good night's sleep, bud.

[ExtremeFiretop]

@Martinski4GitHub

Updated the rights to the URL so you should be able to edit now :)

[ExtremeFiretop]

I've started on Gnuton, feel free to start on the Merlin side!

Goodnight buddy

[Martinski4GitHub]

@ExtremeFiretop,

Hey, bud, just to let you know. I have tested the latest development code as much as I could (given my limited free time), and so far I have not run into any issues.

Also, as mentioned before, my wife & I will take a 14-day vacation starting Thursday, Aug-22, so I'll be pretty much offline until we return. I may check my email occasionally but likely won't respond in a timely manner.

Take care bud. We'll talk when I get back.

[ExtremeFiretop]

@ExtremeFiretop,

Hey, bud, just to let you know. I have tested the latest development code as much as I could (given my limited free time), and so far I have not run into any issues.

Also, as mentioned before, my wife & I will take a 14-day vacation starting Thursday, Aug-22, so I'll be pretty much offline until we return. I may check my email occasionally but likely won't respond in a timely manner.

Take care bud. We'll talk when I get back.

Enjoy the trip my friend, I'll send off the release once I have some free time as well to finish the testing as much as possible. Chat in 14 days! (where are you going btw?)

Hope it's somewhere nice!

[Martinski4GitHub]

@ExtremeFiretop, Hey, bud, just to let you know. I have tested the latest development code as much as I could (given my limited free time), and so far I have not run into any issues. Also, as mentioned before, my wife & I will take a 14-day vacation starting Thursday, Aug-22, so I'll be pretty much offline until we return. I may check my email occasionally but likely won't respond in a timely manner. Take care bud. We'll talk when I get back.

Enjoy the trip my friend, I'll send off the release once I have some free time as well to finish the testing as much as possible. Chat in 14 days! (where are you going btw?)

Hope it's somewhere nice!

We're going to Hawaii. This will be our 2nd time there. The last time was in 2007.

[ExtremeFiretop]

@ExtremeFiretop, Hey, bud, just to let you know. I have tested the latest development code as much as I could (given my limited free time), and so far I have not run into any issues. Also, as mentioned before, my wife & I will take a 14-day vacation starting Thursday, Aug-22, so I'll be pretty much offline until we return. I may check my email occasionally but likely won't respond in a timely manner. Take care bud. We'll talk when I get back.

Enjoy the trip my friend, I'll send off the release once I have some free time as well to finish the testing as much as possible. Chat in 14 days! (where are you going btw?) Hope it's somewhere nice!

We're going to Hawaii. This will be our 2nd time there. The last time was in 2007.

I wish! Stay safe out there on those tropical islands you! ;) Can be lots of storms out there when it's in season, I'm sure you're going to enjoy it!