New definitions for level 7, 8 (and 9)

alexAubin commented 3 years ago

Sooooo this is yet another me-being-obsessed-with-quality-metrics-and-tools™, and kind of a follow up of last meeting where I was mentioning the idea of redefining level 7 and 8.

This PR proposes to add two new "meta" tests.

First one is the "long-term good-quality score", which is computed by analyzing the history of apps.json over the last 12 months and checking how many periods the app was known + flagged working + level >= 5. If that's the case for 90% of the periods, the app is considered as "long-term good quality" (all the parameters are debatable of course - maybe one year is too much ...)

(N.B. : this got updated compared to original post)

A new definition for level 7 and 8 is proposed :

"New" Level 7 = (previous level 7 requirements) + No error / warnings reported by the linter
"New" Level 8 = Maintained + Long-term high quality
"New" Level 9 = Remaining parts of the previous level 8 requirements not covered by the new level 8 (c.f. development workflow, ARM, SSO/LDAP)

Some not-that-important warnings were downgraded to just "Info" to avoid "polluting" the new Level 7 definition. (The 'is_maintained' test is handled in some independent way because it's only part of the level 8 and we don't want to e.g. raise a warning which would interfere with the definition of level 7)

Here are the results when running on all apps :

about 56 apps qualify for the new level 7
about 24 apps qualify for the new level 8 (the flag "maintained" ain't reported in the table, but it might be the reason some apps do not qualify for level 8)

App	# of warnings+errors	Qualify for "new level 7"	Long term good quality	Qualify for "new level 8"
20euros	0	True
243	0	True
adminer	0	True
agendav	0	True
airsonic	3		True
ampache	1
anarchism	2		True
anfora	4
archivist	2		True
backdrop	0	True
baikal	0	True
bibliogram	1
bitwarden	3
blogotext	1		True
bludit	0	True
borg	8		True
borgserver	5
bozon	3		True
cachet	0	True
calibreweb	3		True
cesium	0	True
cheky	3		True
civicrm_drupal7	0	True	True	True
codimd	0	True
couchpotato	5
cowyo	2
cryptpad	1
cubiks-2048	0	True
diagramsnet	0	True
diaspora	1
discourse	2		True
distbin	1		True
dokuwiki	2		True
dolibarr	0	True	True	True
dotclear2	1		True
droppy	0	True
drupal	0	True	True	True
drupal7	0	True	True	True
element	5
etherpad_mypads	2		True
fallback	4		True
ffsync	1		True
firefly-iii	1
flarum	1
fluxbb	1
framaforms	0	True
framagames	2
freshrss	0	True	True	True
friendica	4		True
funkwhale	3		True
garradin	0	True	True	True
ghost	2
gitea	6		True
gitlab	0	True	True	True
gitlab-runner	2		True
glowingbear	1
gogs	7		True
gotify	1		True
grav	0	True	True	True
h5ai	1
halcyon	9
haste	0	True
hextris	0	True	True	True
horde	8		True
hubzilla	2		True
invoiceninja	0	True
jeedom	6
jenkins	8
jirafeau	0	True	True	True
jupyterlab	2		True
kanboard	1		True
keeweb	3		True
kimai2	1
kresus	2		True
leed	5		True
libreto	4		True
limesurvey	7		True
lionwiki-t2t	1
lstu	3		True
lufi	1		True
lutim	3		True
mailman	5		True
mastodon	1		True
matomo	1		True
mattermost	3		True
meilisearch	1
minchat	0	True
mindmaps	0	True
minetest	5		True
mobilizon	0	True
monica	0	True
monitorix	7		True
moodle	1
movim	1
multi_webapp	3		True
mumbleserver	7		True
my-mind	0	True
my_webapp	0	True	True	True
navidrome	1
netdata	2		True
nextcloud	2		True
nodered	1		True
onlyoffice	0	True	True	True
opensondage	2		True
owntracks	0	True
peertube	2		True
petitesannonces	0	True
pgadmin	5		True
phpldapadmin	0	True	True
phpmyadmin	0	True	True	True
phpsysinfo	0	True
pihole	8		True
pilea	0	True
piwigo	0	True	True	True
pixelfed	0	True	True	True
pleroma	1		True
plume	0	True	True	True
pluxml	1		True
prettynoemiecms	0	True	True	True
qr	2		True
radicale	9		True
rainloop	0	True	True	True
redirect	2		True
riot	7		True
roundcube	3
rss-bridge	0	True	True	True
seafile	5		True
searx	3		True
shaarli	3
shellinabox	4		True
simple-torrent	1
slingcode	0	True
snipeit	1		True
sogo	6		True
spftoolbox	1
spip	4		True
streama	1
strut	0	True	True	True
svgedit	0	True
synapse	1		True
syncthing	1		True
thelounge	0	True	True	True
transmission	7		True
ttrss	1		True
tvheadend	4		True
tyto	0	True
unattended_upgrades	1		True
vpnclient	3		True
wallabag2	1		True
webmin	2
webtrees	1
wekan	0	True	True	True
wemawema	0	True
wetty	0	True
wikijs	1		True
wordpress	2		True
writefreely	0	True	True	True
yeswiki	0	True
yunomonitor	4
z-push	5		True
zabbix	7		True
zap	2		True
zerobin	0	True	True	True
ztncui	4

alexAubin commented 3 years ago

Discussed in today's meeting, the proposal is to go for :

move the "non mandatory warning" stuffs as info
define the new level 7 as : (current level 7 checks) + no warnings on the linter
define the new level 8 as : maintained + long term good quality
define the new level 9 as : level 8 actuel

Will run another batch/simulation with this (then also have to clarify/implement the interface with package_check)

alexAubin commented 3 years ago

I updated the original post following the iteration

I decided to keep the checks for chmod777 as warning because looking at corresponding lines in apps, I really think it's a significant issue that could be fixed even though they may not be trivial.
I reworked the "license" check to force the use of an ID listed on spdx (some apps were using just 'free' and saying in the README the upstream is (A)GPL v3 ... it doesn't sound like there's really a usecase of "the-app-is-free-but-with-a-super-custom-license")

Overall there's 61 apps qualifying for the new level 7, and 25 for the new level 8, which sounds reasonable to me.

By moving the not-so-important-warnings into info, it also lowers the number of warnings per app, which means many app could qualify for the new level 7 somewhat easily (just a couple warnings for fix). As discussed in the meeting, one of the goal of this is precisely to create an incentive to fix warnings while at the same time not affecting the display in the catalog (level 6 apps are displayed the same way than level 7)

Josue-T commented 3 years ago

Hello,

About the license I think there are some case not really easy to manage. By example seafile have some different components (ccnet, seahub, seafdav, ...) and each components have a different license so how to manage this ? Actually I wrote all license separated with ',' but the linter don't like it. Maybe we can add support for multiple license ?

Thanks

Le 19 novembre 2020 00:32:01 GMT+01:00, Alexandre Aubin notifications@github.com a écrit :

I updated the original post following the iteration (github ain't displaying the new commits here in the PR ... wtf ... is this some cache issue or something ... they are displayed here though ...)

I decided to keep the checks for chmod777 as warning because looking at corresponding lines in apps, I really think it's a significant issue that could be fixed even though they may not be trivial.

I reworked the "license" check to force the use of an ID listed on spdx (some apps were using just 'free' and saying in the README the upstream is (A)GPL v3 ... it doesn't sound like there's really a usecase of "the-app-is-free-but-with-a-super-custom-license")

Overall there's 57 apps qualifying for the new level 7, and 22 for the new level 8, which sounds reasonable to me.

By moving the not-so-important-warnings into info, it also lowers the number of warnings per app, which means many app could qualify for the new level 7 somewhat easily (just a couple warnings for fix). As discussed in the meeting, one of the goal of this is precisely to create an incentive to fix warnings while at the same time not affecting the display in the catalog (level 6 apps are displayed the same way than level 7)

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/YunoHost/package_linter/pull/87#issuecomment-730022703

-- Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

alexAubin commented 3 years ago

@Josue-T : indeed, I guess having a list separated by commas is fine. This is fixed by the latest commit.

In parallel, while working on the integration with package_check, I decide to flag some error as "Critical" (= app to be capped at level 0) as the mechanism was already here from a while ago but not used, and may also be used in the future for some ideas i have

So far those errors are now considered critical:

missing mandatory field in the manifest
missing yunohost requirement
requiring only yunohost >= 2.x which tends to indicate a package not up to date with recent packaging practices
not being referenced in yunohost's app catalog
not being flagged as "working" in yunohost's app catalog
using deprecated / obsolete helpers like "yunohost app initdb" etc
fetching install arguments using old syntax like domain=$1 which I aim to remove entirely from Yunohost (secrets showing up in ps -ef...)

kay0u commented 3 years ago

@alexAubin

not being referenced in yunohost's app catalog

not being flagged as "working" in yunohost's app catalog

I don't undestand very well, because apps are tested only if they are in the catalog AND if they are flagged as "working"? Or you want to test all apps in the YunoHost-Apps group (plus the ones in the list that are not in the group)?

alexAubin commented 3 years ago

@kay0u : That's related to the "may also be used in the future for some ideas i have" which I was too lazy to explain :stuck_out_tongue_winking_eye:

Basically I'm thinking about including the linter in the core somehow (or a linter-like tool). So if they share the same code that'd be nice ... and such a code would run on the files in /etc/yunohost/apps/$app/. The intention is to report about old/super-custom apps that have super-deprecated practices (for example had the case a few weeks ago because of an app that didnt get upgrade for years) or are not in yunohost's app catalog, or are not flagged as working (maybe because it was flagged as working before, but it's not anymore). Basically all the "critical stuff" (we don't really care about warnings for that use case, it would only be to create more incentive for people to upgrade their outdated apps and discourage using apps not maintained by the community)

kay0u commented 3 years ago

We could add tests like that to the diagnosis system, that sounds great! (But we're going to get a lot of mail about this :-))

Josue-T commented 3 years ago

Hello,

Thanks for your fix. I discovered also a really small issue. I get this error

 ! You should specify a User= directive in the systemd config !

But it's an override file so it don't make sense. It with ffsync.

alexAubin commented 3 years ago

@Josue-T : indeed, this is fixed ! Thanks for the feedback !

YunoHost / package_linter

New definitions for level 7, 8 (and 9) #87