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

Allow the use of separate files for configuration #332

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

Conversation

csegarra
Copy link
Contributor

@csegarra csegarra commented Nov 6, 2017

Instead of using one big Gruntconfig.js file, this patch allows the configuration of basic options on this file and use separate files inside one directory for the configuration of the different tasks grunt-drupal-tasks.

To achieve this we use load-grunt-configs [0]. This module allows you to split your Grunt task configuration objects into separate files any way you choose. With it you also can configure targets for a single task in multiple files.

On the example directory we migrate the configurations of the package task to use this new format, keeping the rest of the configuration on the Gruntconfig.js file.

For using this feature, it only requires to add the key config_dir to the srcPaths object.

[0] https://www.npmjs.com/package/load-grunt-configs

@grayside grayside added the ready label Nov 6, 2017
@grayside
Copy link
Contributor

grayside commented Nov 9, 2017

Hi @csegarra, thank you for your contribution! I need to think about this a bit because my planning around grunt-drupal-tasks configuration handling was more toward decoupling it from GDT itself, instead leaning on generator-gadget to put in place smart configuration loading: phase2/generator-gadget#67

However, before that can be reached, this may be a good interim step.

A couple thoughts:

  1. The src/ directory is reserved for things used by Drupal more actively, and incidentally src/config is being used in Add config directory #330 for Drupal configuration management. In generator-gadget, a grunt plugin loader loads anything found in bin/grunt. Maybe we should have a bin/config? I'm still thinking through the best semantics on this. Maybe it's a top-level grunt/ directory.
  2. Thank you thank you for YAML! :)
  3. Even more thank yous for tests, I will review closer to confirm they are sufficiently deep.

@csegarra
Copy link
Contributor Author

Hi @grayside!
Thank you for the speed of your review and your feedback.

On our first approach (commit 4c12346) we completely remove the Grunconfig.js file and replace it with a config dir on the root of our projects.

For this PR we add the srcPaths.configDir, so the changes we made maintain some backward compatibility for those projects that already use grunt-drupal-tasks.

But... grunt searches the directory on the root directory of the project, not on the src directory... my bad ;)

If you agree, I would move the configDir variable out of srcPaths.

If it's not a config directory on the project's root, I prefer a grunt/config directory, rather than a bin/config. The bin directory usually contains executable files and scripts.

By the way, with the last commit I have updated the npm-shrinkwrap.json to include the load-grunt-configs.

Regards,
Cristian

@grayside grayside added this to the 2.0.0 milestone Jan 8, 2018
unrblt and others added 3 commits May 13, 2021 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants