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 pubip to piactl get command #15

Merged
merged 3 commits into from
Sep 8, 2020

Conversation

MysticRyuujin
Copy link
Contributor

Description
This changes adds pubip to piactl get CLI tool. It fixes #14

Checklist

  • I have described below how to test my code and included unit tests where possible.
  • I have cleaned up the code and made best efforts to match the surrounding coding style.
  • I certify that I have the right to submit this contribution under the GPLv3 and alter as outlined in the Contributor License Agreement.

Testing Plan
I have built this change from source and verified that it returns the Public IP Address as displayed in the GUI

@JonathonH-PIA
Copy link
Contributor

Nice work @MysticRyuujin ! Could you also hook up the appropriate change signal in ValuePrinter::connectValueSignals() (getcommand.cpp) so piactl monitor pubip works?

Also super minor - please tweak the description to sentence case, i.e. Current public IP address 😁

We have a 2.4.0 in flight right now but I'd like to get this in our next release after that! (Shouldn't be any conflicts)

@MysticRyuujin
Copy link
Contributor Author

MysticRyuujin commented Sep 1, 2020

The reason I didn't add it to the ValuePrinter::connectValueSignals was that I couldn't determine what to hook it to...?

vpnIp hooks to &DaemonState::externalVpnIpChanged but what would pubIp hook to?

&DaemonState::externalIpChanged ??

@MysticRyuujin
Copy link
Contributor Author

It looks like that should work, I found the definition of externalVpnIpChanged and there's a matching externalIpChanged 😄

@JonathonH-PIA
Copy link
Contributor

Great work, merging for next release!

@JonathonH-PIA JonathonH-PIA merged commit 3b0250e into pia-foss:master Sep 8, 2020
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.

piactl get pubip
2 participants