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

Remove --sources CLI flag #937

Closed
novemberborn opened this issue Jun 26, 2016 · 8 comments
Closed

Remove --sources CLI flag #937

novemberborn opened this issue Jun 26, 2016 · 8 comments

Comments

@novemberborn
Copy link
Member

With #925 we've changed how test files are resolved. If file patterns are passed as CLI arguments they're resolved relative to the current working directory. Otherwise file patterns in package.json, or indeed default patterns, are resolved relative to the the directory that contains package.json.

Our management of source patterns is similarly affected. This may or may not be the reason why watch mode doesn't work correctly when AVA is called outside of the project root (see #936). However we should reconsider how sources works.

From @jamestalmage in #863 (comment):

If they are cded into a child directory and use the --sources flag, that should probably be resolved from cwd, right? But if they don't specify --sources, then we probably want to use whatever is in package.json, and resolve from that directory? That's getting complicated.

Should we eliminate the --sources flag? I have never used it, and I don't see why you really need it. Doesn't this qualify as a static config item that should live in package.json?

I'm 👍 on removing the sources flag. Patterns must then be specified in the package.json, which hopefully makes them easier to manage. Both for AVA users and maintainers.

@sindresorhus
Copy link
Member

Let's do it.

@novemberborn
Copy link
Member Author

novemberborn commented Sep 23, 2016

Somewhat blocked by #1048.

@novemberborn novemberborn changed the title Remove --sources CLI flag? Remove --sources CLI flag Oct 7, 2016
@sotojuan sotojuan self-assigned this Nov 8, 2016
@sotojuan
Copy link
Contributor

Are we still doing this? Making a list of things to take care of this weekend!

@novemberborn
Copy link
Member Author

@sotojuan yes.

@LasaleFamine
Copy link
Contributor

Hi guys!
Can I take this one? Is there something more I should know?

@novemberborn
Copy link
Member Author

@LasaleFamine go for it! 👍

@LasaleFamine
Copy link
Contributor

Just to be sure, I can see a --source flag, not a --sources. Is right?

Another thing I can see is that the cli.flags.source is used within the Watcher constructor:

// lib/cli.js
if (cli.flags.watch) {
		try {
			const watcher = new Watcher(logger, api, files, arrify(cli.flags.source));
			watcher.observeStdin(process.stdin);
		} catch (err) {

...

We need to get that source prop from the pacakge.json?

@novemberborn
Copy link
Member Author

Just to be sure, I can see a --source flag, not a --sources. Is right?

Yes.

We need to get that source prop from the pacakge.json?

I think you can get it from conf.source. See #1046 for context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants