Repository navigation
[refactor] use async/await syntax? #1867
Description
As I dig through the source, I'm noticing that there's a lot of messy manual Promise chaining, eg. https://github.com/nodegit/nodegit/blob/master/generate/scripts/generateNativeCode.js#L121-L171. Is there a reason not to use async/await syntax? I'd be happy to submit some PRs if maintainers would be interested.
Activity
generate/generateNativeCode.js: async/await and prefer fs over fs-ext…
samuela commentedon Oct 17, 2021
For some context
async/awaitsyntax has been supported in node.js since v7.6.0 which is from 2017, so we wouldn't be dropping support for barely anyone AFAICT.generate/scripts/utils.js: async/await
generate/scripts/generateMissingTests.js: async/await
istanbul -> nyc, use expose-gc
jshint settings: esnext -> esversion
lib/branch.js: async/await
lib/commit.js: async/await
lib/config.js: async/await
lib/merge.js: async/await
lib/rebase.js: async/await
lib/reference.js: async/await
lib/repository.js: async/await
lib/revwalk.js: async/await
lib/tag.js: async/await
lib/tree_entry.js: async/await
lib/tree.js: async/await
lifecycleScripts/submodules/getStatus.js: async/await
lifecycleScripts/submodules/index.js: async/await
lifecycleScripts/postinstall.js: async/await
lifecycleScripts/preinstall.js: async/await
samuela commentedon Oct 18, 2021
I have completed a translation from
.then-ing to async/await for the entirelib/andlifecycleScripts/directories. Commits are linked above. There are only a handful of.thens left in places where they actually appear to be the cleanest solution. Otherwise everything has been refactored to use async/await for clarity.Everything builds and passes the entire test suite. Throughout the process I also upgraded the test runner from istanbul to nyc. It turns out that istanbul has been deprecated, and nyc is the recommended solution now. See samuela@63c801c for more info.
Using async/await syntax also required upgrading jshint, and updating the configuration to use the
"esversion": 9config option instead ofesnextwhich has been deprecated. See samuela@e099639 for more info.I'd be happy to contribute all these changes upstream if they are of interest to maintainers. Let me know! (cc @implausible @ianhattendorf others?)
Vadorequest commentedon Jun 12, 2022
Could this be considered?
jeremyj563 commentedon Dec 13, 2023
Why is this being ignored? Is there some good reason to avoid migrating the project to the modern async/await syntax?