Optimize workflows; Add Dependabot and CodeQL

[thelovekesh]

Currently, there are a few areas where workflows can be optimized to reduce CI times, maintenance, and security in the runners. Also, consider adding tools like @dependabot that can keep the dependency up-to-date unless some dependency requires it, for example: chalk since it's pure ESM now and the plugin CLI is CJS.

Tasks

• [ ] Optimize workflows for lower CI times and reduce files for maintenance(i.e. lint workflows can remain in one file).
• [ ] Remove dependency from third-party workflows for tasks that can be handled natively.
• [ ] Update permissions in the workflows to read-only and manually provide any other permission at the job level.
• [ ] Update wp-env based PHPUnit setup with MySQL + SVN which is easy to set up for any version and has very less or no compatibility maintenance.
• [x] Add @dependabot config to update composer, npm, and github-actions at least once a month.
• [ ] Add CodeQL for JS code analysis for any security vulnerabilities.

[westonruter]

I presume you'll be opening separate PRs for each tasks for review?

[thelovekesh]

Yes, that would be like:

• First three tasks in one PR since they are optimization changes.
• Fourth task in standalone PR since it will refactor PHPUnit setup.
• Last two can be included in single PR since their config files are plain and simple.

[swissspidy]

Some quick thoughts on https://github.com/thelovekesh/performance/pull/2 after taking another look:

• .github/release.yml is not needed. We don't currently use GitHub's auto-generated release notes feature.
• The whole determine-file-counts logic seems a bit overkill and adds complexity. Do we really need that?
• What's the cache busting for. Never had to use something like that in a project before. Why is it needed here?

[thelovekesh]

Thanks, @swissspidy for taking a look. Here are my few observations:

.github/release.yml is not needed. We don't currently use GitHub's auto-generated release notes feature.

Since we are going to add @dependabot && if we ever require to use the GitHub notes release feature, this config can come in handy. We can skip adding it, but I don't see any problem in keeping it too.

The whole determine-file-counts logic seems a bit overkill and adds complexity. Do we really need that?

Currently, there are standalone workflow files for JS and PHP that do the same tasks like linting, testing, etc. Let's say tomorrow we need to add linting for MD files, will we be adding a new workflow file for that? I think it is better to consolidate all linting tasks in a single file, testing tasks in a single file, and so on. The above logic can help determine change paths on the job level which is a well-used thing in multiple projects.

What's the cache busting for. Never had to use something like that in a project before. Why is it needed here?

I think GitHub doesn't delete a cache if it's being used frequently. The above workflow busts that cache weekly.

[swissspidy]

if we ever require to use the GitHub notes release feature, this config can come in handy. We can skip adding it, but I don't see any problem in keeping it too.

It's a misleading to have a config file for a feature that's not being used. It's like dead code. We can just add it when we need it.

I think it is better to consolidate all linting tasks in a single file, testing tasks in a single file, and so on.

Not disagreeing with that part. Consolidating makes sense. And I understand why you added the logic for determining changed files etc. But do we really need it? What's the drawback of not adding it? Will the workflows really be that slower?

[thelovekesh]

What's the drawback of not adding it? Will the workflows really be that slower?

I think it's about running relevant workflows on the changes made in a PR. If the changes are only JS, do we need to run other workflows for like 1-2 minutes(for linting) and maybe 3-4 minutes(for testing)? while the logic for determining changed files only takes 2-3 seconds.

[thelovekesh]

I have found some issues in wp-env based unit tests setup where it seems like Imagick is not working as expected. See https://github.com/WordPress/performance/pull/1013#issuecomment-1962731568

/cc @westonruter @swissspidy @mukeshpanchal27 @joemcgill for visibility

[swissspidy]

Sounds like we can make the tests more robust, good :)

[thelovekesh]

Created https://github.com/WordPress/performance/issues/1085 to discuss task 4 of this issue more widely. cc @swissspidy for visibility.

[adamsilverstein]

I have found some issues in wp-env based unit tests setup where it seems like Imagick is not working as expected. See #1013 (comment)

can we use GD instead of Imagick to address this?

[swissspidy]

I think there are some dedicated unit tests for both GD and Imagick, so ideally we have a setup that has both

[westonruter]

An initial Dependabot config was added by @mukeshpanchal27 in https://github.com/WordPress/performance/pull/1355