ExtremeFiretop / MerlinAutoUpdate-Router

Merlin(A)uto(U)pdate is a Merlin router script which allows you to remotely identify a stable firmware update for an ASUS Merlin router, and automatically download and update via an unattended method directly from the router.
https://www.snbforums.com/threads/merlinau-v1-2-7-the-ultimate-firmware-auto-updater-amtm-addon.91326/
GNU General Public License v3.0
15 stars 1 forks source link

Remove Starting Set_FW_UpdateZIP_DirectoryPath #288

Closed ExtremeFiretop closed 1 month ago

ExtremeFiretop commented 1 month ago

Now that it's no longer a CLI switch/parameter and is instead a combination, we no longer need the Set_FW_UpdateZIPDirectoryPath function at the start.

They will already be in the advanced options menu, if they want to change the directory, they can use the function from the advanced options menu (option 1)

It was only originally included when the design was to be a CLI entry-point. (Feels a little weird now to have it as the starting function now)

ExtremeFiretop commented 1 month ago

@Martinski4GitHub

This is just a suggestion for now, I don't really want to do another regular release just for this. So if you approve this PR. Just approve it, don't merge it.

I'll do the change in the Gnuton branch instead.

Martinski4GitHub commented 1 month ago

Now that it's no longer a CLI switch/parameter and is instead a combination, we no longer need the Set_FW_UpdateZIPDirectoryPath function at the start.

They will already be in the advanced options menu, if they want to change the directory, they can use the function from the advanced options menu (option 1)

It was only originally included when the design was to be a CLI entry-point. (Feels a little weird now to have it as the starting function now)

Yes, that makes sense and it's a very good point. Great catch!!

Martinski4GitHub commented 1 month ago

@Martinski4GitHub

This is just a suggestion for now, I don't really want to do another regular release just for this. So if you approve this PR. Just approve it, don't merge it.

Yes, of course, there's no need for another release. This does not affect the regular update process anyway so it would affect only one user (that we know of ?? LOL!!). It's not an urgent fix either.

ExtremeFiretop commented 1 month ago

@Martinski4GitHub This is just a suggestion for now, I don't really want to do another regular release just for this. So if you approve this PR. Just approve it, don't merge it.

Yes, of course, there's no need for another release. This does not affect the regular update process anyway so it would affect only one user (that we know of ?? LOL!!). It's not an urgent fix either.

Absolutely not, no regular user is impacted your correct on that front. It's not urgent or even required. The reason was that our user that helped with the feedback, tested it.

And was a bit confused by the starting prompt. He didn't understand why he needed to change the directory, (which he didn't) and then he tried to upload the firmware directly to the home/root folder instead of waiting until the next screen that said to upload the firmware now.

That's when I realized that entire screen isn't required anymore. If your in the advanced menu now, you do the combination, your expecting to see right away the directory to upload the zip, not the option 1 function to change/confirm the directory.

ExtremeFiretop commented 1 month ago

I should mention that other than that, everything worked perfectly!

ExtremeFiretop commented 1 month ago

Approved & good to go.

Thank you for approving. I'll do the change in Gnuton now and close this PR. (As discussed)

ExtremeFiretop commented 1 month ago

@Martinski4GitHub

Updated in Gnuton commit: https://github.com/ExtremeFiretop/MerlinAutoUpdate-Router/pull/186/commits/91dbcb4cfacb6628596cb1d457c75ce17c69c6ac

(Will be included in the Gnuton merge)