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

Recommend a setup method for CI that only needs to be done once and never maintained or updated #323

Closed
fulldecent opened this issue Oct 8, 2024 · 10 comments

Comments

@fulldecent
Copy link

Current documentation says to

Copy gha.dist.yml file to .github/workflows directory in your project.

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

@gjb2048
Copy link
Contributor

gjb2048 commented Oct 8, 2024

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.

@gjb2048
Copy link
Contributor

gjb2048 commented Oct 8, 2024

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

@gjb2048
Copy link
Contributor

gjb2048 commented Oct 8, 2024

@fulldecent
Copy link
Author

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.

@fulldecent
Copy link
Author

And also IT REMOVES THEIR GREEN CI PASSED BADGE if a new version is released which the plugin author does not support!

@gjb2048
Copy link
Contributor

gjb2048 commented Oct 8, 2024

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

@fulldecent
Copy link
Author

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.

@gjb2048
Copy link
Contributor

gjb2048 commented Oct 9, 2024

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

@fulldecent fulldecent changed the title Change recommended usage method Recommend a setup method for CI that only needs to be done once and never maintained or updated Oct 9, 2024
@fulldecent
Copy link
Author

Updated issue title to accurately reflect the specific concern

@paulholden
Copy link
Member

Closing as duplicate of #303 - thanks!

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

No branches or pull requests

3 participants