Decaffeinate, add new features and possible API breakage #125
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
Using decaffeinate npm module. Clean up has yet to come. Delete coffee files. Update .npmignore, .editorconfig and package.json Add .eslintrc.json Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Some urls and dependencies need to be updated. Signed-off-by: Alexandre Demers <[email protected]>
Our code will now have to use ES module coding style. Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Introduce an API handler function which returns the right instance of AutoLaunch API implementation according to the platform it is running on. The API class and subclasses will be moved under their own folder in a latter commit, once we are ready to use this function. Signed-off-by: Alexandre Demers <[email protected]>
An empty string was able to pass through. Catch all falsy values. Signed-off-by: Alexandre Demers <[email protected]>
…API implementation Separate the code under a basic AutoLaunchAPI class and implement per platform child classes. This way, we can have specific code coherent with each platform, based on prior code. Since we are now working with classes, an instanciated object can refer to its own parameters using "this". Move the modules under library folder and its subfolder. Fix some more decaffeinated code. Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Fix some anonymous functions using "fat arrow" to access "this". Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
The "mac" object has been relocated under "options" wirth the new API. Signed-off-by: Alexandre Demers <[email protected]>
Technically, someone could use the --experimental-modules flag with nodejs version 12. But since we are going to bump the major version, we can upgrade our dependency requirements. Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
This needs to be handled by and for Linux and FreeBSD specifically. It takes care of path when AppImage packages are used. It also escapes the path properly for its use in the Desktop entry file. Signed-off-by: Alexandre Demers <[email protected]>
This needs to be handled by and for macOS specifically. Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Can't push() without an array. Signed-off-by: Alexandre Demers <[email protected]>
The instance knows about itself and can refer to its own parameters. Signed-off-by: Alexandre Demers <[email protected]>
In the decaffeination, it seems I mixed the mkdirp() call usage. No callback, but a then(). Clarify style because oneliner blocks are a hell to spot and read. Move anonymous "function" name to using "fat arrow" (leftovers). Signed-off-by: Alexandre Demers <[email protected]>
Move some anonymous "function" to using "fat arrow" (leftovers). Better class the tests between platforms. POSIX tests should be run on both Linux/FreeBSD and macOS platforms since they rely on writting and reading files on POSIX filesystems. Signed-off-by: Alexandre Demers <[email protected]>
Eventhough my local testing tells me otherwise when testing, there is no .opts member anymore. Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Still working on local Linux machine. Will it work in CI? Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Because we are back at the start where done() catches a non-error pointing at ~/.config/autostart Signed-off-by: Alexandre Demers <[email protected]>
Maybe this will do the trick. Signed-off-by: Alexandre Demers <[email protected]>
The problem was not at all where I was looking for it. The error was arising on CI because the autostart folder must not exists. Thus, createFile() was wrongly throwing an error after calling mkdirp. Mkdirp returns a string if a folder is created (the first one created if recursive) or undefined if the folder already exists. In case of error, it rejects the Promise. Since Node fs can manage mkdir recursively itself, let's use the native call and use the callback to manage the error/rejection if any. Fix catch(done) to catch failures properly. Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
… tests We were leaving garbages behind us when testing. Make sure we clean it up. To allow proper cleanup, move the failure tests all together so they don't mix up with the after/afterEach call. POSIX: add some tests to ensure we properly honor appName with the Desktop entry file.
Signed-off-by: Alexandre Demers <[email protected]>
To help solve issue 122, improve the information logged about the app's path and any error message when registring or unregistring the registry key. (see Teamwork#122) Signed-off-by: Alexandre Demers <[email protected]>
Signed-off-by: Alexandre Demers <[email protected]>
Actually bump major version for RC preparation Signed-off-by: Alexandre Demers <[email protected]>
…o Oxalin-decaffeinate Fix remaining issue from decaffeination and other improvements.
|
Sorry for the long commit list. A lot of the commits are related to a bug I was experiencing remotely through CI. I was unable to reproduce locally until I figured out it was because of a folder that was not existing by default. Major changes:
|
This was
linked to
issues
Mar 22, 2024
Closed
|
Could you add this as a branch under the Teamwork/node-auto-launch repo rather than your own fork? That way tests will run on the pr and i can push changes directly to the branch. |
|
Yes I could do that. |
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.
This removes all CoffeeScript in favor of JavaScript and EM6 module.
Actually add AppX support.
Improve Linux support.
Improve test coverage.
API breakage: to better support extra arguments to be added at launch and to better structure the options, the API is changed. While it should still support the old options, I may have missed some cases.