Create Task
Maniphest T7667

Script that checks the minimal requirements for a pull request
Open, Needs TriagePublic
Actions

Description

Having a script that contains default checks could be run before a contributor does a PR:
* checking copyrights are updated
* qsTr() are used

  • no Text but GCText
  • no temp files
  • author name of the commit is "FirstName SecondName"
  • each ActivityInfo has qsTr(), createdInVersion and an intro text

...
Check http://gcompris.net/wiki/Reviewing_an_activity and http://gcompris.net/wiki/GSOC_newcomers#Code_quality to see what else could be added and the previous github PR that contains a lot of same comments.

jjazeix created this task.Dec 31 2017, 1:12 PM
jjazeix assigned this task to echarruau.Dec 31 2017, 1:41 PM
jjazeix updated the task description. (Show Details)Feb 4 2018, 12:19 PM
jjazeix moved this task from Backlog to WIP on the GCompris: Improvements board.Feb 21 2018, 4:02 PM
harrymecwan added a subscriber: harrymecwan.Feb 21 2018, 5:00 PM

Anyone working on this ? need help to get started on this task.

jjazeix added a comment.EditedMar 14 2018, 8:48 AM

we can also add the different version of Qt imports (QtQuick should be 2.6...)

jjazeix updated the task description. (Show Details)Apr 9 2018, 7:07 AM
echarruau added a comment.Apr 20 2018, 2:55 PM

Could you move the tools into tools directory? And also create a test able to extract the element name when we find a string "......".
You can use the regexp I gave you for that. Thanks.

harrymecwan added a comment.Apr 29 2018, 9:29 PM

Yeah, sure but what element name should i test it against ?

echarruau added a comment.Apr 30 2018, 2:20 PM

if you use the regexp I gave you it will detect all the " ...... " patern (anything between quotation marks).
In a first time there will be lots of false error.
But the we can use the groups to identify the element nature to know if we display it or not.

echarruau added a comment.Apr 30 2018, 3:02 PM

To help the other reader than harry to read this thread, the regexp is

matchStr = /([^ ]+): "[^"]+"/.match(lineStr)

echarruau added a comment.Apr 30 2018, 3:48 PM

I have added a few lines to create some element exceptions.
If there are string without qstr and If the elements are in a defined list (see code) there is no warning
I attach the diff from your actual code.

diff.txt981 BDownload

jjazeix added a comment.EditedMay 16 2018, 12:50 PM

use of "var" instead of "variant" in qml

echarruau updated the task description. (Show Details)Aug 15 2018, 6:22 AM
echarruau added a comment.Aug 15 2018, 7:00 AM

Created branch checkActivitiesScripts into kde repository.
The file check_activities_ids_to_escape.txt contains the id elements name to escape when checking if qstr is missing.
check_activities.rb is the main file, it can be used with the "all" option to test all the activities in alpphabetical order.

harrymecwan updated the task description. (Show Details)Sep 10 2018, 6:13 PM
jjazeix added a comment.May 14 2022, 9:10 AM

For info, as we switched to gitlab, I think we can add a new "rule" in https://invent.kde.org/education/gcompris/-/blob/master/.gitlab-ci.yml to add the run of the script on MR