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

UpdateTunnel deck to 1.0.4 #689

Merged
merged 4 commits into from
Oct 22, 2024

Conversation

bkohler616
Copy link
Contributor

@bkohler616 bkohler616 commented Sep 5, 2024

TunnelDeck

  • Added a "NetworkInfo" section that informs you of what the prioritized network interface is.
  • Additionally, provides ping-ability to both the gateway and steam, as well as ping times and general network info that's useful for debugging. This information is periodically collected.

Checklist:

Developer Checklist

  • I am the original author or an authorized maintainer of this plugin.
  • I have abided by the licenses of the libraries I am utilizing, including attaching license notices where appropriate.

Plugin Checklist

  • I have verified that my plugin works properly on the Stable and Beta update channels of SteamOS.
  • I have verified my plugin is unique or alternatively provides more/alternative functionality to a similar plugin already on the store. (The original TunnelDeck is technically unique on it's own)

Plugin Backend Checklist

  • No: I am not using a custom backend other than Python.
  • No: I am not using a tool or software from a 3rd party FOSS project that does not have it's dependencies statically linked.
  • No: I am not using a custom binary that has all of it's dependencies statically linked.

Testing

  • Tested on SteamOS Preview Update Channel.

@bkohler616 bkohler616 requested a review from a team as a code owner September 5, 2024 15:29
@bkohler616 bkohler616 changed the title Tunnel deck 1.0.4 UpdateTunnel deck to 1.0.4 Sep 5, 2024
Copy link
Member

@TrainDoctor TrainDoctor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkohler616 please make sure to go back through package.json and or plugin.json and properly denote the plugin's original author and to remove his name from the email which you have changed over to your own. I have no problem allowing this PR to continue if this is done. Once this is done the plugin will be good to proceed to the testing store.

@TrainDoctor TrainDoctor requested a review from a team September 5, 2024 16:03
@TrainDoctor TrainDoctor added the needsfix An issue/change request needs to be resolved label Sep 5, 2024
@bkohler616
Copy link
Contributor Author

My bad on that one - he has been properly added as a contributor, and I guess I'm the "author" now. If you want me to set him back as the author I can, just lemme know.

@TrainDoctor
Copy link
Member

My bad on that one - he has been properly added as a contributor, and I guess I'm the "author" now. If you want me to set him back as the author I can, just lemme know.

If Steve wishes to retake the project then we differ to the original developer within our discussion so all good atm.

@TrainDoctor TrainDoctor requested review from a team September 5, 2024 18:11
Copy link
Member

@TrainDoctor TrainDoctor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good for testing.

@AAGaming00
Copy link
Member

small suggestion to reduce shutdown times (applies to previous versions too): you really don't need to restart networkmanager on shutdown when the openvpn extension isn't enabled in the first place

@TrainDoctor TrainDoctor removed the needsfix An issue/change request needs to be resolved label Sep 12, 2024
@bkohler616
Copy link
Contributor Author

small suggestion to reduce shutdown times (applies to previous versions too): you really don't need to restart networkmanager on shutdown when the openvpn extension isn't enabled in the first place

Gotcha, I'll have that done in next version.

@bkohler616
Copy link
Contributor Author

bkohler616 commented Oct 22, 2024

Hiya. Couple of things.

  1. Is my testing results sufficient to allowing this to push through? If not, may someone help me out in testing this guy please :)
  2. @AAGaming00 , you meantioned that on shutdown that I'm resetting the Network Manager on shutdown - I'm assuming you mean the _unload function is running the uninstall script? If not, then I'm a bit lost on your suggestion, as the NetworkManager is only being restarted on enable / disable of ipv6.

Here's my testing report if that's valid or not:
A) No major issues.
B) When accessing the plugin quick-setting without internet the list of VPN's does not show up for 20 seconds.
C)

  • TunnelDeck - 1.0.4-244f6a2
  • MoonDeck - 1.7.1
  • PowerTools - 2.0.3
  • EmuDecky - 1.0.8-cca33c1

D) Yay*

    • for all uses except ipv6 or openVPN - I do not have a valid setup for either of these, so I am unable to properly test these cases.

@AAGaming00
Copy link
Member

yes it's the uninstall script

Copy link
Member

@TrainDoctor TrainDoctor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testers approve! Off to production.

@TrainDoctor TrainDoctor merged commit 0786e2d into SteamDeckHomebrew:main Oct 22, 2024
2 checks passed
@bkohler616 bkohler616 deleted the tunnel-deck-1.0.4 branch October 22, 2024 18:16
@TrainDoctor
Copy link
Member

@bkohler616 apologies but I just realized that you were the original PR author and I didn't realize that. In future you do need another user to properly fill out the testing form in future.

@bkohler616
Copy link
Contributor Author

@TrainDoctor Sorry, I didn't mean to rush you on that front - my apologies. I also don't really have known people that can test for me (I kinda suck at talking 😅)

I put up a new one, so hopefully I can get it right this time. I'll poke the community for a tester that may wanna help me out.
#712

And again, super sorry my man :(

@TrainDoctor
Copy link
Member

@TrainDoctor Sorry, I didn't mean to rush you on that front - my apologies. I also don't really have known people that can test for me (I kinda suck at talking 😅)

I put up a new one, so hopefully I can get it right this time. I'll poke the community for a tester that may wanna help me out. #712

And again, super sorry my man :(

Nothing to apologize for though I appreciate the thought. It's on me to better review the comments in these PRs and verify that proper procedure is being followed. You even asked whether this testing report was valid in the comment lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants