-
Notifications
You must be signed in to change notification settings - Fork 19
Display the address of the server #258
base: master
Are you sure you want to change the base?
Display the address of the server #258
Conversation
src/views/drawer.html
Outdated
<div class="drawer" ng-class="{visible:drawerVisible}"> | ||
<div class="drawerLead" style="text-align: center; color: darkblue"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be constant styles in the HTML directly. Add a class and use CSS for the styling
server_modules/address.js
Outdated
var address = interfaces[i][j];//we basically parse over all the addresses for every network interface | ||
if (address.family === 'IPv4' && !address.internal) {//if that address is an IPv4 address, and it is not an internal one, we add it | ||
addresses.push(address.address); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only use the first you find, but you continue to iterate over all of them. Use break or add the condition into the loop condition. Also you can keep only one in memory, but that's not really important
Not sure about this one. Some thoughts:
|
@@ -13,7 +13,8 @@ var middlewareLayers = [express.static(fileSystem.resolve('src')), | |||
require('./server_modules/cors').middleware, | |||
require('./server_modules/cache').middleware, | |||
require('./server_modules/body_builder').middleware, | |||
require('./server_modules/log').middleware]; | |||
require('./server_modules/log').middleware, | |||
require('./server_modules/address').middleware]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to place the closing bracket on its own line, to prevent unnecessary changes to the last line (e.g. how the log
line is now changed).
var os = require('os'); | ||
var args = require('./args') | ||
|
||
//shamelessly copied from StackOverflow with only minor changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this is simple code, copying stuff without a clear license may conflict with the license of this project.
<div class="drawer" ng-class="{visible:drawerVisible}"> | ||
<div class="drawerLead"> | ||
address: {{serverAddress}}</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong indentation: the text should be indented, the closing tag on its own line.
@alanggreen Can you give an update on your internal discussions about this? |
@poelstra Discussed the idea of putting the address/port information in the tray with Roy. It's a possibility but not at this moment as he is focused on getting the installer up and running. That is my number 1 priority at the moment. If he can get that done then we can ask @ThinkRedstone to make this change. |
solves #242 .
Colour of the address is subject to change; To me, it seems the easiest to see.
I don't know if there's a better way to display the server IP without adding a new middleware and adding a new file.
address is not a very good name for that file, should probably be changed, but I have no idea for a better name.