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

Networks page: Properly show hotspot interface #269

Merged
merged 2 commits into from
May 28, 2020

Conversation

andre8244
Copy link
Contributor

@andre8244 andre8244 commented May 28, 2020

  • Display hotspot interface with a proper icon and a distinctive color
  • Show tun-dedalo CIDR address

NethServer/dev#6186

@nethbot
Copy link
Member

nethbot commented May 28, 2020

in 7.8.2003/autobuild:

Copy link
Member

@gsanchietti gsanchietti left a comment

Choose a reason for hiding this comment

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

I have mixed feelings on this one.
While the patch for sure reaches the goal, you're hard-coding stuff from an extra package (nethserver-dedalo, not widely used) straight into the core.

IMO this is not acceptable from an architecture point of view

@andre8244
Copy link
Contributor Author

I have mixed feelings on this one.
While the patch for sure reaches the goal, you're hard-coding stuff from an extra package (nethserver-dedalo, not widely used) straight into the core.

IMO this is not acceptable from an architecture point of view

You are right, I didn't think about this architectural aspect. Is your concern about the tun-dedalo CIDR retrieval only, or to the hotspot role management as a whole? Not showing hotspot CIDR could be a good compromise?

@gsanchietti
Copy link
Member

You are right, I didn't think about this architectural aspect. Is your concern about the tun-dedalo CIDR retrieval only, or to the hotspot role management as a whole? Not showing hotspot CIDR could be a good compromise?

The hotspot role as a whole, I didn't never liked it. Still, it's there since NethServer 6 and the role didn't change the name so I think it's stable enough.
Let's go with displaying just not displaying the CIDR, so you do not need to hard-code tun-dedalo interface name ;)

@nethbot
Copy link
Member

nethbot commented May 28, 2020

in 7.8.2003/autobuild:

@edospadoni edospadoni merged commit 1a7bf02 into NethServer:master May 28, 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
4 participants