Skip to content

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Sep 6, 2023

@kenjis kenjis added the github_actions Pull requests that update GitHub Actions code label Sep 6, 2023
@kenjis kenjis marked this pull request as draft September 6, 2023 06:57
@kenjis kenjis marked this pull request as ready for review September 6, 2023 08:40
Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

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

Honestly, I always have a problem with one repo being dependent on another. However, I understand the benefits of this work for the team. You are trying to eliminate duplicate tasks.

codeigniter4/.github It seems to be public, and I assume so any repo can use this method even if it is not under organization?

Co-authored-by: Pooya Parsa <pooya_parsa_dadashi@yahoo.com>
@kenjis
Copy link
Member Author

kenjis commented Sep 7, 2023

codeigniter4/.github It seems to be public, and I assume so any repo can use this method even if it is not under organization?

Yes.

The called workflow is stored in a public repository, and your organization allows you to use public reusable workflows.
https://docs.github.com/en/actions/using-workflows/reusing-workflows#access-to-reusable-workflows

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Even though I think using Rector as a non-dependency is technically better I think this ends up being cleaner and safer. I will move my own work in the same direction.

@kenjis kenjis merged commit 40cd12b into codeigniter4:develop Sep 9, 2023
@kenjis kenjis deleted the reuse-workflows branch September 9, 2023 20:03
@kenjis
Copy link
Member Author

kenjis commented Sep 9, 2023

@MGatner If we don't have rector, devs cannot run rector.
And PhpStorm always complains about "Undefined class" in rector.php.

rector.php is closely related to the version of rector; we cannot specify the version of rector without adding rector to composer.json.

Have you experienced any dependency conflicts with installing rector?

@MGatner
Copy link
Member

MGatner commented Sep 9, 2023

Nope! I bought into Bergmann's strong anti-dependency sentiment for static analysis when I first created this. I'm over it 😇 This is a good change.

@kenjis kenjis mentioned this pull request Sep 12, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants