search

YunoHost / package_linter

Linter for YunoHost applications packages
https://yunohost.org/#/packaging_apps
GNU Affero General Public License v3.0
17 stars 13 forks source link

[enh] Add warning and some test #31

Closed zamentur closed 6 years ago

zamentur commented 7 years ago

Problem

Some test were not accurate (exit and set -eu) A lot of YEP are not tested.

Suggested solution

This PR implements a warning level and a wrong but no sure level. It had some new test, some of them are not so linked to the role of a linter but it test some YEP, feel free to move it in an other place. Remove all print that say "ok", just display the failing test

How to test

just run package linter on some apps

PR Status

Opinion needed Might need a refactoring

Validation

M5oul commented 6 years ago

Thanks for the PR. The diff is unreadable. Could you split the work in several commits explaining the behaviour modification coming with the diff?

Psycojoker commented 6 years ago

You can look at this PR as an example of "split every step in a clear commit to make this reviewable" https://github.com/YunoHost/yunohost/pull/353/commits

maniackcrudelis commented 6 years ago

However, I can say it works fine on our official CI since 3 weeks 👍.

alexAubin commented 6 years ago

We already don't have enough manpower to review legitimate PRs which are reviewable, so I find it really weird to point out that the PR is unreadable (while the diff is readable, imho - at least compared to some other PR in the past) and expect the OP to just rewrite the PR... I doubt it's gonna happen and we can't afford it anyway. Especially since the PR is apparently already in prod and working.

M5oul commented 6 years ago

Ok, go ahead. I let you take the lead of package_linter. I don't have the energy to review this one in detail.