Skip to content

Chore: New checklist structure#938

Open
Daetha wants to merge 3 commits intomainfrom
dev/new-pr-checklist
Open

Chore: New checklist structure#938
Daetha wants to merge 3 commits intomainfrom
dev/new-pr-checklist

Conversation

@Daetha
Copy link
Copy Markdown
Collaborator

@Daetha Daetha commented Feb 10, 2026

The new checklist is attached to this PR description. What does the team think? Let's discuss in the comments.

Changes

[What changes have you made?]

Notes for Reviewer

[Describe the steps needed to test this]

Checklist

My code is styled, reviewed and understandable

  • PHP code is documented using PHPDoc.
  • JavaScript code is documented using JSDoc.

My code is documented

  • ELMO Guide (./doc/help.html) has been updated.
  • README has been updated.
  • API documentation (./api/v2/docs) has been updated.
  • changelog (./doc/changelog.html) has been updated.

My code is productive

This PR is covered with the tests. Tests are added first for code that runs often, has a high failure impact, or is difficult to validate through manual testing.

  • PHPUnit
  • JS
  • Playwright
  • My changes do not create new warnings in the test browser console.
  • My code meets accessibility guidelines.

Known Issues

[What lower priority problems remain?]

Known Issues

[What lower priority problems remain?]

@Daetha Daetha self-assigned this Feb 10, 2026
@McNamara84 McNamara84 added the Abstimmung ausstehend Wird im nächsten Meeting diskutiert label Feb 11, 2026
@McNamara84
Copy link
Copy Markdown
Owner

Hi @Daetha ,
hi @Ali-GeoFZ ,

for me it is fine, when you both agree with this changes. You both are currently the responsible devs for the ELMOs and if you are both happy with thist checklist it is okay for me.

However I would like to make some suggestions:

  • For me it would be easier ti have a checklist whre I have to check all checkboxes when the PR is ready for review. This is the reason why I wrote some checklist points in the current with "If applicable, ...". Then you have to check if appicable and you can check the checkbox anyway.
  • For me Code Coverage is very important. To cover not all changes and new function or features can lead to a low coverage but only the most called can lead to a low coverage over all.

Only suggestions from my pouint of view.

Best,

Holger

@Daetha Daetha requested a review from Ali-GeoFZ February 11, 2026 11:40
@Ali-GeoFZ
Copy link
Copy Markdown
Collaborator

Dear @Daetha and @McNamara84,
I don't consider PR to be a high priority and it can wait until next week, as we will be meeting then. I am currently reviewing all of our issues (handover is next week). In my opinion, this checklist should also be approved by the product owner.

@McNamara84 McNamara84 changed the title Developer things: new checklist structure Chore: new checklist structure Feb 25, 2026
@McNamara84 McNamara84 changed the title Chore: new checklist structure Chore: New checklist structure Feb 25, 2026
Copy link
Copy Markdown
Collaborator

@Ali-GeoFZ Ali-GeoFZ left a comment

Choose a reason for hiding this comment

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

Hi @Daetha,

as discussed, I talked with our product owner, and she is fine with us updating the checklist. I’ve now prepared my version, which keeps all of the original points but groups them into clearer sections e.g., Code Quality, Documentation, and Testing.

This structure should make it easier to see what’s needed for a PR before review, while still keeping the flexibility of the “if needed” items you mentioned.

Feel free to have a look at my suggested version below. If you and @McNamara84 both agree with these changes, it’s okay for me, too.

Best regards
Ali

Pull Request Checklist

Code Quality

  • My code follows the project’s style guide.
  • I have self-reviewed my code.
  • I added comments for hard-to-understand code.
  • My changes do not create new warnings in the test browser console.

Documentation

  • If applicable, PHP code is documented using PHPDoc.
  • If applicable, JavaScript code is documented using JSDoc.
  • If needed, the ELMO Guide (./doc/help.html) has been updated.
  • If needed, the README has been updated.
  • If needed, the API documentation (./api/v2/docs) has been updated.
  • If a new feature was added or a bug fixed, the changelog (./doc/changelog.html) has been updated.

Testing

  • If needed, Playwright tests have been updated or added.
  • If needed, unit tests have been updated or added.
  • If needed, jest tests have been updated or added.

@Daetha Daetha requested a review from Ali-GeoFZ March 23, 2026 10:54
@Ali-GeoFZ
Copy link
Copy Markdown
Collaborator

Dear @Daetha, before I approve this pull request, which checklist did we agree on?

@Daetha
Copy link
Copy Markdown
Collaborator Author

Daetha commented Mar 23, 2026

Dear @Daetha, before I approve this pull request, which checklist did we agree on?

The one with your improvements ; )

@Ali-GeoFZ
Copy link
Copy Markdown
Collaborator

Dear @Daetha, before I approve this pull request, which checklist did we agree on?

The one with your improvements ; )

But you haven't updated the local checklist yet :)

@Daetha Daetha requested review from McNamara84 and removed request for Ali-GeoFZ April 2, 2026 10:21
@Daetha Daetha dismissed Ali-GeoFZ’s stale review April 2, 2026 10:22

Suggestions applied

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Abstimmung ausstehend Wird im nächsten Meeting diskutiert

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants