Skip to content

Conversation

@sarjona
Copy link
Member

@sarjona sarjona commented Jan 24, 2022

Bumped PHP version to 7.3.0 and PHPUnit version to 9.5.* to meet Moodle requirements.
Apart from that, for now, the sabberworm/php-css-parser version has been fixed to 8.1.* because 8.2 onwards has a bug with
comments (raised by a couple of PHPUnit tests).

Besides, Github Actions support has been added to RTLCSS for PHP, using the recommended configuration.

The result of the GHA jobs in my local repository can be checked in https://github.com/sarjona/rtlcss-php/actions/workflows/ci.yml

I've also added the image in the README.md file (which won't work until the GHA job will be run by the first time).

Bumped PHP version to 7.3.0 and PHPUnit version to 9.5.* to meet
Moodle requirements.
Apart from that, for now, the sabberworm/php-css-parser version
has been fixed to 8.1.* because 8.2 onwards has a bug with
comments (raised by a couple of PHPUnit tests).
@sarjona sarjona mentioned this pull request Jan 24, 2022
@stronk7
Copy link
Member

stronk7 commented Jan 24, 2022

Nice fixes!

Some little comments, copied from elsewhere:

  1. Only detail I can imagine is why in GHA you “restrict” the phpunit execution to tests/MoodleHQ/RTLCSS/RTLCSSTest.php . Given it has a phpunit.xml, it should be able to find all the tests without specifying any.
  2. Also the matrix.extensions seems to be unused (surely also the max_input_vars). They are only needed for moodle, so far.
  3. And a Fixes #3 anywhere in the commit message will, automatically, close Unit tests won't run #3
  4. Finally, surely after all the latest fixes and this... we'll need to do a 1.0.1 release or so (I imagine). To be incorporated to core.

Ciao :-)

Add Github Actions to RTLCSS for PHP, using the recommended
configuration. Fixes moodlehq#3.
@sarjona
Copy link
Member Author

sarjona commented Jan 24, 2022

Hi @stronk7 !
Thanks for reviewing the patch! Good catches!!

  1. Removed tests/MoodleHQ/RTLCSS/RTLCSSTest.php, so now any PHPUnit test should be executed.
  2. matrix.extensions and max_input_vars removed because you were right, they weren't used.
  3. Added Fixes Unit tests won't run #3 to the commit message (I wasn't aware of that... or didn't remember it!)
  4. I was planning to release a version once this will be merged and then upgrading the library in core using MDL-72626.

@stronk7
Copy link
Member

stronk7 commented Jan 24, 2022

Sold!

@stronk7 stronk7 merged commit e251ac7 into moodlehq:master Jan 24, 2022
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

Successfully merging this pull request may close these issues.

2 participants