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

Feat: Configuration via cosmiconfig #34

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

Conversation

apoleshchuk
Copy link

Useful for directory based configuration.

Closes #25

Copy link
Owner

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request! This is very cool!

I'm thinking, shoulnd't we just remove the old config and make a new major release? Doesn't make much sense two have two types of config files.

src/index.js Outdated
return Object.assign(
{},
getConfigFromFile(directories, filename),
configFromCosmiconfig || {},
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't we call getConfigFromCosmiconfig here the same way we do the other two options?

Copy link
Author

Choose a reason for hiding this comment

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

I wanted call in same way, but config required early in mrm.js for custom dir, preset name.

One call for read config — potentially better performance.

Use only cosmiconfig for read config files — sound good, i try to refactor code.

Copy link
Owner

Choose a reason for hiding this comment

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

For the preset name, I guess you can swap the blocks const options... and const preset....

bin/mrm.js Outdated
searchPlaces: ['config.json']
});
directories.push(presetDir);
options = Object.assign({}, presetConfig, options);
Copy link
Author

Choose a reason for hiding this comment

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

It's not clearly code, but useful for shareable configs via presets, like aliases or company based configs.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure what this code is doing, could you please add comment?

Copy link
Author

Choose a reason for hiding this comment

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

Shareable configs via preset useful for teams, sample <preset>/config.json:

{
	"emailValidator": "([\\w\\.]+)@corp\\.mail\\.ru",
	"any-shareable-config": "some-value",
	"aliases": {
		"component": ["license", "readme", "editorconfig", "gitignore", "husky"]
	}
}

as default config for task, that can rewrite by developer config, if needed.

Copy link
Owner

Choose a reason for hiding this comment

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

Could you add some docs for this use case?

@ismay
Copy link

ismay commented Jun 3, 2019

This seems to have stalled a bit. It seems very useful though. Have these changes already been imported elsewhere, or are we waiting for the requested changes?

@sapegin
Copy link
Owner

sapegin commented Jun 3, 2019

@ismay I need to have a look again — I remember I had some doubts how it should work, but don't remember exactly ;-£

What's your use case? Do you have any comments on the implementation?

@ismay
Copy link

ismay commented Jun 3, 2019

What's your use case? Do you have any comments on the implementation?

Not yet. I need to test mrm out a bit more before I can make any useful comments. But as far as configuration goes I think cosmiconfig is the gold standard, so in my opinion it'd be nice to use it.

@danielkcz
Copy link

danielkcz commented Jul 26, 2019

I haven't tried MRM in practice yet, but what I am currently trying to achieve is to be able to run a single command without specifying any details and based on a config file within a project it would execute tasks in a custom preset. In the preset repo, I want to have a base config which would have an alias to basically run all available tasks.

Additionally, I want to provide a task or something that will utilize some sort of prompt module to configure the project initially.

I think it makes sense to go out with the next major version that uses cosmiconfig only.

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.

Read configuration from project directory
4 participants