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

Feature Request: please add support for grumphp.yml.dist file #104

Closed
keradus opened this issue Feb 4, 2016 · 12 comments
Closed

Feature Request: please add support for grumphp.yml.dist file #104

keradus opened this issue Feb 4, 2016 · 12 comments
Milestone

Comments

@keradus
Copy link
Contributor

keradus commented Feb 4, 2016

No description provided.

@veewee
Copy link
Contributor

veewee commented Feb 4, 2016

Can you explain why we need a grumphp.yml.dist file?
The idea is to add the same git hooks for every user so that the same rules are applied for every user.

@keradus
Copy link
Contributor Author

keradus commented Feb 4, 2016

If one team decided to store configuration file on repository it's cool.
But then sb will want to have a bit different configuration, and he can't.
.dist file is file to be distributed with repository, non-dist file is private one.

Real-user-cases:

  • team decide to use tasks A+B, but sb want to run C as well.
  • configuring things like stashing depending on individual needs

@lukaszwit
Copy link

I think that "grumphp.yml.dist" is not grumphp concern. If you want it you can add it to your project and commit to your repo and smiply add grumphp.yml to .gitignore

@keradus
Copy link
Contributor Author

keradus commented Feb 4, 2016

That's exactly the plan. Exactly the same usage like let say phpunit.xml and phpunit.xml.dist.
But, following phpunit example, when you have .xml file phpunit will use it by default, and if not then phpunit will try to find .xml.dist file without the need of manually providing the configuration file.

Same thing would be awesome for grumphp, to use grumphp.yml if exists, and if not try to use grumphp.yml.dist.

@lukaszwit
Copy link

@keradus I get you point and it seems easy implement
Personally I would love to see similar mechanism that symfony has with their parameters.yml and parameters.yml.dist (parameters.yml is generated or updated after composer install/update command)

@veewee
Copy link
Contributor

veewee commented Feb 4, 2016

Well I am not fully convinced: As I mentioned, GrumPHP is used for assuring the same coding standards, code quality, ... When you depend on a tool for this, you don't want all users to possibly have their own config. By allowing user specific config's, you will get situations where you expect that a certain tool has run, but it hasn't because the user has overwritten the configuration.

The idea is to let GrumPHP handle the rules you've decided as a project / team. If somebody wants to contribute to that project, he should play it by the rules that were globally set.

I agree that it isn't a lot of work to implement, but I am looking for a valid use-case that is in the line of the ideology of the tool.

On the other hand: there are already a lot of ways to trick the system (commit -n, custom config files for the tool it runs, ...)

@keradus
Copy link
Contributor Author

keradus commented Feb 4, 2016

What about the stashing thing?

@veewee
Copy link
Contributor

veewee commented Feb 4, 2016

that might be a valid option. We don't want people come in and complain about GrumPHP is eating their code when they enabled the stash option ...

@keradus
Copy link
Contributor Author

keradus commented Feb 4, 2016

Actually the same thing for verbose. One like small, one like big. Even when running same ruleset.

@veewee
Copy link
Contributor

veewee commented Feb 5, 2016

Good point. The only concern I have is that people will add / remove tasks so that the checks that should run aren't running during a commit. But I don't think that is something we can enforce anyway.

@veewee
Copy link
Contributor

veewee commented Mar 30, 2016

I am working on the dist support:
veewee@9583413

Can you verify the locator to make sure that this is how it should work?
I will finish this PR when I find some more time :)

@veewee veewee added this to the v0.9.0 milestone Mar 30, 2016
@veewee
Copy link
Contributor

veewee commented Apr 4, 2016

The dist file is added in PR #135. I also changed the default value of the ignore_unstaged_changes option to false. More info about the issue in #123.

@veewee veewee closed this as completed Apr 4, 2016
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

3 participants