Skip to content

Conversation

@fluvf
Copy link
Contributor

@fluvf fluvf commented Jul 18, 2019

See commit messages for some details.
Also adds hash checking of downloaded file

This is what I meant by removing console spam in #45. console.log() messages could be added when these operations start and end in the functions that call them, rather than in the functions themselves

@nickrnet
Copy link
Member

Ok, you should hold up a minute. I have a bunch of changes incoming for async/await goodness 😃 I'll get it finished up probably tomorrow and then I'll try to merge some of these in.

@fluvf
Copy link
Contributor Author

fluvf commented Jul 18, 2019

Sorry :D This is currently the only one that is ready for review, the other ones are still WIP

@fluvf fluvf changed the title MinecraftServer: Move server download and version fetching logic into its own file WIP MinecraftServer: Move server download and version fetching logic into its own file Aug 4, 2019
@fluvf fluvf changed the title WIP MinecraftServer: Move server download and version fetching logic into its own file Server download and version fetching logic in its own file Aug 14, 2019
@fluvf
Copy link
Contributor Author

fluvf commented Aug 14, 2019

I did some fixes and added proper tests
This doesn't replace any logic in MinecraftServer.js anymore, as I'll mess around with it in upcoming pull requests anyways

By the way, the reason I use path.resolve when requiring these classes, is because they expose static properties. That way all other scripts that require them (using the absolute path), would also "see" if those properties were to be modified. I don't think I'll be using that feature anytime soon, but who knows, maybe later down the line.
Just so you know! Node does a good job with platform compatibility, so using path isn't always strictly necessary, but it is good practice.

@nickrnet
Copy link
Member

During my experiments with NPM module testing, the path.resolve or path.join statements in the require's were not working properly. The require module seems to handle the forward slash / right regardless of platform.

But I understand the static property stuff too... we'll have to see how we can get this sorted another way.

@fluvf
Copy link
Contributor Author

fluvf commented Aug 15, 2019

The solution might be to use __dirname, instead of path.resolve, as it doesn't use the current working directory

@fluvf fluvf changed the title Server download and version fetching logic in its own file WIP - Server download and version fetching logic in its own file Aug 25, 2019
@fluvf fluvf changed the title WIP - Server download and version fetching logic in its own file WIP - Move server version management logic into its own file Sep 7, 2019
Might aswell while we're here
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.

2 participants