Gizra / drupal-starter

Drupal 10 Starter with best practices
76 stars 40 forks source link

Issue-693: Disallow scheduler on locked pages (if scheduler module is enabled) #754

Open sonvir249 opened 1 week ago

sonvir249 commented 1 week ago

Closes #693

sonvir249 commented 1 week ago

@amitaibu @mgurjanov Just a suggestion, shouldn't we add github actions to catch the drupal php and twig code stanrdard, theme build issues whenever any PR is rasied or commit is pushed. I have integrated Twig-CS-Fixer as mentioned in [dependency evaluation] Adopt vincentlanglet/twig-cs-fixer for Twig coding standards

mgurjanov commented 1 week ago

@sonvir249 We do run ddev phpcs or ddev phpstan locally prior to pushing code to github.

sonvir249 commented 1 week ago

@sonvir249 We do run ddev phpcs or ddev phpstan locally prior to pushing code to github.

I also do that on local, but having it in the pipeline adds an extra layer of assurance to ensure nothing goes wrong with the code.

amitaibu commented 1 week ago

ddev phpcs or ddev phpstan

They are already part of the Travis CI pipieline

@sonvir249 could you also add a test case please?

mgurjanov commented 1 week ago

@amitaibu I was thinking of asking for test case, but wouldn't that require installing and enabling scheduler?

amitaibu commented 1 week ago

I was thinking of asking for test case,

@mgurjanov always go with that gut feeling πŸ˜„

but wouldn't that require installing and enabling scheduler?

We can have it installed with composer, and the test would enable it. In the future, we could decide to also enable it by default - depends if we see more real world cases where clients need it.

mgurjanov commented 1 week ago

@amitaibu It's funny how it always fires back to me when I don't, and I do follow gut feeling most of the time. Thanks for the reminder!

amitaibu commented 1 week ago

πŸ˜† πŸ‘Œ

sonvir249 commented 1 week ago

ddev phpcs or ddev phpstan

They are already part of the Travis CI pipieline

@sonvir249 could you also add a test case please?

@amitaibu Sure, I can handle that. Could you help clarify what areas I should cover in the tests? Also, I don’t have much experience writing test cases, so any references or guidance would be really helpful. Thanks!

amitaibu commented 5 days ago

You can confirm, for example, that the form changes have kicked in.

You can see an existing test here