Skip to content

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

Merged
merged 2 commits into from
Nov 18, 2021
Merged

Templates #1

merged 2 commits into from
Nov 18, 2021

Conversation

MGatner
Copy link
Member

@MGatner MGatner commented Nov 17, 2021

  • Adds workflow templates
  • Adds config files examples for workflows
  • Updates docs with a lot more info

@MGatner
Copy link
Member Author

MGatner commented Nov 17, 2021

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?

@lonnieezell
Copy link
Member

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.

@MGatner
Copy link
Member Author

MGatner commented Nov 17, 2021

@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 Settings and Tasks. It means that the PHPUnit 8.0 tests run twice but it gives us the option of include/excluding the infection.yml workflow in places where we don't want it. I would definitely like to see it included in this repo, as most people have not been exposed to "test quality tools", but I'm fine removing it from existing libraries for now.

@lonnieezell
Copy link
Member

@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.

@MGatner
Copy link
Member Author

MGatner commented Nov 17, 2021

@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.

@samsonasik
Copy link
Member

@MGatner for rector, I suggest to use pinned version, eg: 0.12.4:
https://github.com/codeigniter4/CodeIgniter4/blob/d4db7b22dc8fb9901d94ab89c14f84fc662fff0e/composer.json#L27

and use dependabot to verify if it still working with next version.

@MGatner
Copy link
Member Author

MGatner commented Nov 17, 2021

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?

@samsonasik
Copy link
Member

@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 CodeIgniter4's repo:

https://github.com/codeigniter4/CodeIgniter4/blob/develop/rector.php

@MGatner
Copy link
Member Author

MGatner commented Nov 18, 2021

@samsonasik I will work on something and get you to review it.

Copy link
Member

@kenjis kenjis left a 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.

@MGatner
Copy link
Member Author

MGatner commented Nov 18, 2021

@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.

@MGatner MGatner merged commit 761d524 into develop Nov 18, 2021
@MGatner MGatner deleted the templates branch November 22, 2021 01:18
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.

4 participants