-
Notifications
You must be signed in to change notification settings - Fork 46
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
Recommend a setup method for CI that only needs to be done once and never maintained or updated #323
Comments
But Moodle 4.1 is still supported in terms of security, https://moodledev.io/general/releases, so PHP 7.4 is still a valid entry. |
@fulldecent In the example you've given it states Moodle 3.5, https://github.com/catalyst/catalyst-moodle-workflows?tab=readme-ov-file#configure-support-range, which is even older than the one stated on #322 (Moodle 3.8.3) - so I don't get what you're getting at by these issues. |
Also https://github.com/catalyst/catalyst-moodle-workflows/blob/main/.github/actions/matrix/matrix_includes.yml goes as far back as M3.3 with PHP7.1! |
The versions of Moodle that are supported is a separate issue. And Catalyst is not cited as best practice here. Sorry, you are right, PHP 7.4 must be supported. But in terms of install method, the approach that Catalyst is using is DRY and could be considered best practice: # .github/workflows/ci.yml
name: ci
on: [push, pull_request]
jobs:
ci:
uses: catalyst/catalyst-moodle-workflows/.github/workflows/ci.yml@main
# Required if you plan to publish (uncomment the below)
# secrets:
# moodle_org_token: ${{ secrets.MOODLE_ORG_TOKEN }}
with:
# Any further options in this section because that would allow the CI to always the recommended version that Moodle HQ has published. |
And also IT REMOVES THEIR GREEN CI PASSED BADGE if a new version is released which the plugin author does not support! |
@fulldecent What evidence and peer review statements do you have to assert the statement of 'considered best practice'? In my experience as a plugin developer, having that badge or not won't make much difference. The only difference that users tend to detect is if the plugin works or not. And if a given plugin supports a given version of Moodle in the plugins database. |
DRY is best practice. It has it's own Wikipedia page https://en.wikipedia.org/wiki/Don%27t_repeat_yourself I recognize that Catalyst implements DRY in their install instructions. They implement this by recommending people to use their CI workflows by reference. Separately, and unrelated to this issue, Catalyst may have some policy about which Moodle versions they support. By linking to Catalyst and saying their CI workflow is an implementation of DRY best practice, I do intend to say that Catalysts's Moodle version policy is best practice. In fact Catalyst's Moodle version policy is not best practice. Moodle's official version support is documented at https://moodledev.io/general/releases and can be considered best practice. We have a separate discussion issue open here to discuss Moodle versions. I'm sorry for the confusion I caused on these points by using the words "best practice" twice above referencing two different things. And I also caused confusion with my error at top about PHP versions. IMHO as a plugin author, and my experience trying to make plugin development easier, is that plugins should be easier to make, and easier to keep up-to-date. IMHO as a plugin user, any plugin which does not support the latest released version of Moodle should be shown at the bottom of any search result and any page related to that plugin should have a yellow warning badge at the top of it. And any plugin which does not support any of the any of Moodle's currently supported releases should be automatically removed from the Moodle plugin directory. |
@fulldecent I see. Where is there repetition in what is here? (i.e. where do things repeat themselves?). Just because Catalyst have a policy, doesn't mean that Moodle HQ should follow it, as they have one perspective for their business and Moodle HQ will have theirs. Yes, plugin development should be easier. However changing this tool won't make the difference. The work comes from coping with the core API and underlying technological changes. There is, I believe, a discussion about the Plugin's database, and that will be separate to this. |
Updated issue title to accurately reflect the specific concern |
Closing as duplicate of #303 - thanks! |
Current documentation says to
This is not DRY and it can lead to outdated flow. For example, this file supports old PHP versions, which should be removed. If you had copied that file in your plugin then your workflow is years outdated, and will continue to be years outdated until somebody on the team randomly thinks to compare that copy-pasta with upstream.
Please study and implement this approach as a primary or a secondary recommendation along with that
https://github.com/catalyst/catalyst-moodle-workflows?tab=readme-ov-file#add-the-workflow
The text was updated successfully, but these errors were encountered: