Auto Increment Port #263
Closed
Auto Increment Port #263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alallier
requested changes
Sep 29, 2020
There was a problem hiding this comment.
Something makes my spidey senses tingle by auto incrementing an explicit port in an application automatically with no choice at all.
I'm inclined to only allow auto incrementing a port on the command line side of reload (which currently has a PR open #262) but I think there is a possible argument to be made that reload is development only feature and auto incrementing the port in a development environment can make things easier. I'd like to call on @kethinov and @Autre31415 for some second opinions here.
Depending on the result of this conversation the following action items required are:
- Put the auto increment behind an option and provide a console log stating that an auto increment occurred and what the new port reload is running on
- Re-run the GitHub Actions and fix errors
- Write tests
| @@ -57,7 +57,7 @@ module.exports = function reload (app, opts, server) { | |||
| } | |||
|
|
|||
| // Application setup | |||
| setupClientSideCode() | |||
| // setupClientSideCode(port) | |||
There was a problem hiding this comment.
This is being done below in your changes so please remove the old call here
|
Some thoughts:
|
alallier
added a commit
that referenced
this pull request
Nov 19, 2020
… the port if it is already in use. This commit adds the following to the existing work: * Added new tests * Added the port to reload's return object so the calling program knows what port reload ending up using if the port auto increments * Added autoIncrementPort optional param to realod to turn off auto port incrementing. Default `true` * Tweaked the code to work correctly, like handling the promise return correctly * Small tweaks to how the generated client side code replace works. Removed a branch that wasn't hitting in coverage and made it more readable * Updated README with API changes
|
Closing in favor of #278 |
alallier
added a commit
that referenced
this pull request
Nov 21, 2020
… the port if it is already in use. This commit adds the following to the existing work: * Added new tests * Added the port to reload's return object so the calling program knows what port reload ending up using if the port auto increments * Added autoIncrementPort optional param to realod to turn off auto port incrementing. Default `true` * Tweaked the code to work correctly, like handling the promise return correctly * Small tweaks to how the generated client side code replace works. Removed a branch that wasn't hitting in coverage and made it more readable * Updated README with API changes
alallier
added a commit
that referenced
this pull request
Nov 21, 2020
… the port if it is already in use. This commit adds the following to the existing work: * Added new tests * Added the port to reload's return object so the calling program knows what port reload ending up using if the port auto increments * Added autoIncrementPort optional param to realod to turn off auto port incrementing. Default `true` * Tweaked the code to work correctly, like handling the promise return correctly * Small tweaks to how the generated client side code replace works. Removed a branch that wasn't hitting in coverage and made it more readable * Updated README with API changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Silently bumps the IP address if taken. Quick fix for several of the reported issues.