Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add WireGuard VPN Interfaces #61

Merged
merged 4 commits into from
Mar 1, 2025
Merged

Conversation

ExtremeFiretop
Copy link

@ExtremeFiretop ExtremeFiretop commented Mar 1, 2025

Add WireGuard VPN Interfaces.

image
image
image

This is an additional change to @Martinski4GitHub 's PR: 60
Just figured today seems WireGuard is the more prominent VPN solution, and it's an easy addition.

Wanted to add a quick unrelated note that the develop branch of spdMerlin has been working well for some time :)
My 2 cents is it's ready to merge to master branch.

Martinski4GitHub and others added 4 commits August 19, 2024 02:11
Removed/modified code related to "var $j = jQuery.noConflict();" which is now considered obsolete.
@jackyaz jackyaz merged commit ea2998b into jackyaz:develop Mar 1, 2025
1 check passed
@Martinski4GitHub
Copy link

Martinski4GitHub commented Mar 2, 2025

@ExtremeFiretop,

I just updated my forked repo of spdMerlin, took a quick look at your PR for the latest changes, and noticed some errors in the JavaScript file that's currently in the repository. See screenshot below:

spdstats_www _JS_Errors

As you can see, the same pair of variables is defined 6 times, which is most likely not your intention.

Can you double-check this code and verify that the JavaScript that got embedded into the ASP file is the correct version and the file in the repository gets fixed?

Thanks.

@jackyaz
Copy link
Owner

jackyaz commented Mar 2, 2025

Do i need to unpick the changes?

@Martinski4GitHub
Copy link

Do i need to unpick the changes?

Well, it looks like the minified version of the JS file that got embedded into the ASP has that part of the code corrected, but I haven't reviewed the rest. My impression is that what we have is 2 versions of the JS file: the one in the repository and the one that got minified and embedded, and they don't match; but the minified version seems to be working.

Anyway, you could roll back the changes now, or wait for @ExtremeFiretop to respond.
He's usually pretty good at replying within a few hours of getting the message.

My 2 cents.

@ExtremeFiretop
Copy link
Author

ExtremeFiretop commented Mar 2, 2025

Do i need to unpick the changes?

Well, it looks like the minified version of the JS file that got embedded into the ASP has that part of the code corrected, but I haven't reviewed the rest. My impression is that what we have is 2 versions of the JS file: the one in the repository and the one that got minified and embedded, and they don't match; but the minified version seems to be working.

Anyway, you could roll back the changes now, or wait for @ExtremeFiretop to respond. He's usually pretty good at replying within a few hours of getting the message.

My 2 cents.

@Martinski4GitHub

You are absolutely correct in your assessment. The minified version of the JS file that got embedded into the ASP has that part of the code corrected. Everything is actually functional, however I totally forgot to go back and update the actual .js file with the changes to the values I used!!!

Will quickly whip up a correction to the master .js file in a moment.

@ExtremeFiretop
Copy link
Author

ExtremeFiretop commented Mar 2, 2025

Bingo! #62

Donezo; sorry about that mixup! The original master .js file was supposed to be updated once I got the minified version working ;)

@ExtremeFiretop
Copy link
Author

Do i need to unpick the changes?

Anyway, you could roll back the changes now, or wait for @ExtremeFiretop to respond. He's usually pretty good at replying within a few hours of getting the message.

My 2 cents.

I do pride myself on making myself available, but you happened to find my mixup while I was catching a few hours of zzzz's!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants