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

fix: replace usage of outdated ifconfig with ip #129

Merged
merged 4 commits into from
Jan 23, 2025

Conversation

BoboTiG
Copy link
Contributor

@BoboTiG BoboTiG commented Jan 4, 2025

Fixes #41.

  • Changes in int2ip(), and ip2int(), are cosmetic only.
  • Tested with success on my local machine (Debian 13) + Vultr (Ubuntu 24.04).
  • Removed macOS-specific code since the installer now only supports Ubuntu.

@HDauven HDauven requested review from herr-seppia and HDauven January 5, 2025 00:01
Copy link
Member

@HDauven HDauven left a comment

Choose a reason for hiding this comment

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

Thanks again @BoboTiG!

LGTM. I think we can remove some of the check_installed calls with this change.

I would like @herr-seppia his input though, since he wanted to keep some of the changes in here before.

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jan 22, 2025

Yes, I guess we can remove dnsutils.

Copy link
Member

@herr-seppia herr-seppia left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -12,43 +12,25 @@ if [[ -z "$PUBLIC_IP" || ! "$PUBLIC_IP" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]];
exit 1
fi

runOnMac=false
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the fact that MacOS support was removed, but indeed it's out of node-installer scope

Copy link
Member

@HDauven HDauven Jan 23, 2025

Choose a reason for hiding this comment

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

I don't like the fact that MacOS support was removed, but indeed it's out of node-installer scope

There's a couple of pathways we can take. The easiest is to implement #27 if we use MacOS with the installer for dev purposes. Or we try to actually broaden support once everything is more "stable" in the future.

node-installer.sh Outdated Show resolved Hide resolved
@HDauven HDauven merged commit 9cdf0be into dusk-network:main Jan 23, 2025
@BoboTiG BoboTiG deleted the fix-41-outdated-ifconfig branch January 23, 2025 09:22
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.

ifconfig is outdated
3 participants