gulpjs / liftoff

Launch your command line tool with ease.
MIT License
843 stars 52 forks source link

feat!: Add `extends` syntax #103

Closed phated closed 2 years ago

phated commented 5 years ago

@sttk this is my (very hacky) initial work on my vision for extends syntax. Please review and make any suggestions. I still need to find answers on all the TODOs and write a ton of tests (which will likely result in refactoring).

Closes #101 (supersedes it)

sttk commented 5 years ago

@phated This pr is generally good! I'm sorry that I'm slow on the uptake. Wrong idea (and my poor english) had hindered my understanding. As you commented, findUp option of fined can solve the issue of loading a config file in cwd found up.

BTW, there are one thing I'd like you to confirm. Will the way to specify .configFiles of constructor be changed to use array? (I think it's better for those order.)

phated commented 5 years ago

@sttk don't worry about it! Your work heavily influenced this code.

BTW, there are one thing I'd like you to confirm. Will the way to specify .configFiles of constructor be changed to use array? (I think it's better for those order.)

I think it's important to switch the configFiles values to be array because we are now loading and merging the configs. My current implementation will look at each entry until one is found, then it will load that file and switch to merging using extends property.

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1356541201


Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/array_find.js 8 9 88.89%
lib/needs_lookup.js 6 7 85.71%
index.js 43 45 95.56%
<!-- Total: 57 61 93.44% -->
Totals Coverage Status
Change from base Build 1356472823: -2.04%
Covered Lines: 247
Relevant Lines: 251

💛 - Coveralls
phated commented 2 years ago

@sttk this has taken me a long time to come back to, but I am hoping you can review this PR because it adds the extends syntax that we plan to support in gulp-cli. Thanks 🙏

phated commented 2 years ago

Oh, and this is a breaking change because we changed the syntax to an array of fined objects (instead of a single object).

sttk commented 2 years ago

@phated OK, I'll review this PR this weekend.