-
Notifications
You must be signed in to change notification settings - Fork 14
Templates #1
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
Conversation
MGatner
commented
Nov 17, 2021
- Adds workflow templates
- Adds config files examples for workflows
- Updates docs with a lot more info
Hey team, please look over these well as I would like these to trickle down to our existing repos. @samsonasik would you like to take a shot at a suitable Rector version? |
All in all it looks fine, and I appreciate the updated readme! My only question is - do we need Infection to run constantly? I get that it can be a good tool to use when updating your tests, but I feel it introduces a lot of extra noise when checking pull requests. As far as I'm concerned, as long as we make it easy to run manually, it doesn't need to be included on a PR. Curious to see what others think, though. |
@lonnieezell I picked up on that from our last Infection conversation 🤗 I've actually moved it to its own workflow, which is a change from what is live on |
@MGatner I agree it has a place in this repo. And I'd love to be able to run it manually whenever. Just don't love it when looking over PRs, that's all. :) And, to be fair, until you started using it here, I wasn't familiar with it either. |
@lonnieezell No worries! Right now there is no template injection or anything, so files supplied here are for developers to copy in as they choose. Eventually I want to include a script to integrate these into repos, especially to merge updates, but for now I suggest that we publish these (once the team has had a chance to review) and then incorporate them manually into our current repos. |
@MGatner for rector, I suggest to use pinned version, eg: 0.12.4: and use dependabot to verify if it still working with next version. |
Thanks @samsonasik. Would the repo's Config file be appropriate as a general configuration for other projects & libraries? Or is there a more generic version we can push? This also brings up the point: should we delay publishing this repo until PHP 7.4 is the minimum and we can use more specific tooling? |
@MGatner I am not sure, if the "Config" you mean is the "rector.php" configuration, that possibly better to have per-project basis, like in current https://github.com/codeigniter4/CodeIgniter4/blob/develop/rector.php |
@samsonasik I will work on something and get you to review it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see all the details of the workflow settings, but it looks generally good.
@kenjis since these are just recommendations and not usable directly it isn't so important that we get the workflows perfect. That said these are based on my own templates that I've been using and tweaking for many months, took this opportunity to clean them up and consolidate them. I'm excited to be offering this officially alongside the framework, instead of it always be my own little corner. |