Update mariadb plugin

[uberhacker]

Bare minimum self-checks

What do you think of a person who only does the bare minimum?

• I've updated this PR with the latest code from main
• I've done a cursory QA pass of my code locally
• I've ensured all automated status check and tests pass
• I've connected this PR to an issue

Pieces of flare

• I've written a unit or functional test for my code
• I've updated relevant documentation it my code changes it
• I've updated this repo's README if my code changes it
• I've updated this repo's CHANGELOG with my change unless its a trivial change (like updating a typo in the docs)

Finally

• I've requested a review with relevant people

If you have any issues or need help please join the #contributors channel in the Lando slack and someone will gladly help you out!

You can also check out the coder guide.

[netlify]

✅ Deploy Preview for lando-backdrop ready!

Name	Link
🔨 Latest commit	df8bb3d
🔍 Latest deploy log	https://app.netlify.com/sites/lando-backdrop/deploys/663298610bc5820007cb6505
😎 Deploy Preview	https://deploy-preview-47--lando-backdrop.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 94 (no change from production) Accessibility: 96 (no change from production) Best Practices: 100 (no change from production) SEO: 93 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[xurizaemon]

Not a maintainer, but I'd recommend to make separate PRs for the mariadb update and the text changes, rather than combining changes into one.

Text cleanup is helpful but mixing changes in PRs can hide unexpected change.

Same thoughts as last review lando/acquia#95 (comment) on including version number and release date in CHANGELOG.

[xurizaemon]

This looks like legal text, suggest caution on changes.

[uberhacker]

What do you mean by "legal text"? Just fixing a spelling error.

[xurizaemon]

I mean it doesn't look like regular English, but "legalese". We can see that if this is "just a spelling error", it's one propagated through many Lando plugins (search for "softwares" shows many results) and that file appears to be a legal document.

As a fellow contributor and maintainer of other projects, I recommend to propose changes "atomically", ie changing only one thing at a time. Mixing a version bump with changes to legal text can complicate review, or might have downstream effects if that "spelling mistake" actually affected something. I would guess not ... but it's so much easier to focus scope of a PR and be certain there are no unexpected side-effects for the project.

If "softwares" really needs fixing, it would make more sense to fix that across the dozens of places it appears in Lando plugins in one go, rather than ad-hoc alongside other changes.

As noted: I'm not a maintainer here, so it's up to Lando team to make the call!

[uberhacker]

I see what you mean now. This brings up another question. Why are we making changes in dozens of places instead of change one and use anywhere?

[uberhacker]

More on the reason for the proposed change: https://thewordcounter.com/plural-of-software/

[xurizaemon]

This thread is now well outside the scope of "Update mariadb plugin".

I recommend moving changes unrelated to the subject of the PR to a separate change. IMO the changes in this PR which do belong here are those proposed to:

• .github/workflows/pr-backdrop-tests.yml
• CHANGELOG.md
• examples/backdrop-mariadb/.lando.yml
• examples/backdrop-mariadb/README.md
• examples/backdrop-mariadb/web/index.php
• examples/backdrop-mariadb/web/info.php
• package-lock.json
• package.json

Changes to those files above look good to me, based on comparison with existing examples and other plugins. The rest I propose to remove from this PR.

It's great to tidy as you go - I respect the good scouting attitude! - but mixing different changes is a complication that can be avoided. Moving unrelated changes to separate PRs allows you to still do that tidying as you go; git add -p (and git branches) are effective tools to do this.

The first couple paragraphs (and screenshot!) in https://www.netlify.com/blog/2020/03/31/how-to-scope-down-prs/ may explain this better than I am managing to do?

[uberhacker]

@xurizaemon: Moved other changes to #49. Appreciate your feedback. This brings up another question. How do we handle multiple pull requests in the same release? Obviously, we don't want multiple entries in CHANGELOG.md. Any suggestions @reynoldsalec?

[xurizaemon]

Nice. I'm happy with the changes here now, keen to see tests run which requires a manual action by Lando core team I think.

On the CHANGELOG, yeah I agree. Picking release numbers and dates is out of scope for a PR (that really only works for a maintainer who is cutting releases immediately). A format that works better for contribution is something like https://keepachangelog.com with changes landing in an "Unreleased" section, leaving release management for the maintainer later.

That's a discussion for a separate issue and not specific to this repo, IMO.

[uberhacker]

Agree this should be a maintainer responsibility. Maybe the tagged release message can be appended to CHANGELOG.md as part of the process? Typically a release message would include a summary line item for each commit/PR along with a diff of changes from the previous release. This looks interesting.

[xurizaemon]

I've opened lando/lando#3710 for this, let's discuss there.

[uberhacker]

Not a maintainer, but I'd recommend to make separate PRs for the mariadb update and the text changes, rather than combining changes into one.

Text cleanup is helpful but mixing changes in PRs can hide unexpected change.

Same thoughts as last review lando/acquia#95 (comment) on including version number and release date in CHANGELOG.

So are you suggesting the version should be 1.3.1 instead of 1.4.0? #51 mentions to update the docs. I figured a codespell check wouldn't hurt to clean up.

[xurizaemon]

So are you suggesting the version should be 1.3.1 instead of 1.4.0? lando/mariadb#51 mentions to update the docs. I figured a codespell check wouldn't hurt to clean up.

More "I don't know, so I recommend to check what the correct next version and publication date will be before making that assertion" than "I think it should be different". Maybe that happens in the PR discussion 😄

Comparing https://github.com/lando/drupal/blob/main/CHANGELOG.md I suspect your version change to 1.4.0 is the correct one.

Personally, I would probably leave the date out of the CHANGELOG because I wouldn't want to guess at the release date. (For me it's easier to keep an "Unreleased" section at the top of the CHANGELOG, and let the maintainer make the updates when releases are cut, but that varies per project ... if Lando style is for the proposer to add a date, so be it!). See #47 (comment)

Re "x says to update the docs" - agree, but update the docs within the scope of the proposed changes IMO. For general tidy up changes, a separate "Docs improvements" PR is what I'd aim for.

Hope this helps!

[xurizaemon]

LGTM, awaiting test run.

[reynoldsalec]

@uberhacker I think the error on the mariadb test is from using the mysql binary...I guess we'd have to add mariadb as a tooling command (similar to how we add mysql: https://github.com/lando/backdrop/blob/main/utils/get-tooling-defaults.js#L45

[uberhacker]

@uberhacker I think the error on the mariadb test is from using the mysql binary...I guess we'd have to add mariadb as a tooling command (similar to how we add mysql: https://github.com/lando/backdrop/blob/main/utils/get-tooling-defaults.js#L45

This should be ready to go now @reynoldsalec. All tests passed.