Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better config resolving #37

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tunnckoCore
Copy link
Contributor

@tunnckoCore tunnckoCore commented Jun 6, 2018

resolves #4 and deprecates #36 and #29 ;d

So what is the final result? There is a comment, but let's describe again.

If someone want one global config file they don't need to do anything or to use a --maidfile flag - it just works. It will look 5-10 directories up to find such file, otherwise you should pass path to a markdown file (e.g. README.md or ~/myconfigs/maidconfig.md) which has h2 header and tasks names as h3 headers, the optional use of <!-- maid-tasks --> still work though.

Charlike Mike Reagent added 2 commits June 6, 2018 10:57
- rewrite resolving mechanism
- add `yarn maid` script
- add implementations of egoist#36
- replace occurances pf `node bin/cli` with `yarn maid` because it uses the --maidfile flag

Signed-off-by: Charlike Mike Reagent <[email protected]>
*/
module.exports = (opts = {}) => {
let filepath =
find.sync(MAIDFILE, opts.cwd, 5) || find.sync(`.${MAIDFILE}`, opts.cwd, 10)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh.. i forgot that, but it's not needed for find-file-up to work it uses the cwd by default anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use of the --cwd flag is not encouraged anyway, because it also may conflict with other CLIs (#35)

Charlike Mike Reagent added 2 commits June 6, 2018 11:30
Signed-off-by: Charlike Mike Reagent <[email protected]>
Signed-off-by: Charlike Mike Reagent <[email protected]>
@tunnckoCore
Copy link
Contributor Author

tunnckoCore commented Jun 6, 2018

Btw, last commit may look strange, but is required and this problem probably comes from the cac that. Without that change, it was passing the README.md to all tasks.

you can see here how there is .bin/eslint . README.md lint and node_modules/.bin/ava README.md test that's what i'm talking in #35 and how it may be a problem. Think for example if some of the CLI has lint command too, what we should do then?

❯ yarn test
yarn run v1.7.0
$ yarn maid lint && yarn maid test
$ node bin/cli --maidfile README.md lint
[11:32:07] Starting 'lint'...
$ /home/charlike/tunnckoCore/maid/node_modules/.bin/eslint . README.md lint
[11:32:10] Finished 'lint' after 2318 ms...
$ node bin/cli --maidfile README.md test
[11:32:11] Starting 'test'...
$ /home/charlike/tunnckoCore/maid/node_modules/.bin/ava README.md test

  5 passed
[11:32:12] Finished 'test' after 1703 ms...
Done in 6.04s.

Definitely this should somehow be handled by cac. Should pass them after --? So to be eslint . -- README.md lint and only pass the flags passed to maid before the --?

Like for example running lint task

maid lint --fix

should represent

eslint . --fix -- README.md lint

?

That's why #35 is reasonable and mostly sure it may appear as problem in future, and users should be caution when use maid.

@tunnckoCore
Copy link
Contributor Author

TODO: update docs.

@jakepearson
Copy link
Collaborator

@tunnckoCore, I have access to start merging, would you fix the conflicts and I will merge today. Thanks.

*/
module.exports = (opts = {}) => {
let filepath =
find.sync(MAIDFILE, null, 5) || find.sync(`.${MAIDFILE}`, null, 10)
Copy link
Owner

@egoist egoist Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about simply using loadFile? It by default will stop searching files when reaching /.

loadFile.loadSync(['maidfile.md', '.maidfile.md'])
// not sure why we need to support .maidfile.md though

And remove the stopDir here:

stopDir: path.dirname(process.cwd())

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.

Global location for maid file
3 participants