Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to create issues on Github repo that will flag the app as broken or low quality #77

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexAubin
Copy link
Member

@alexAubin alexAubin commented Nov 30, 2019

There are numerous issues that are too specific or too complex to be detected by the linter. For example :

  • Apps installing docker but not removing it correctly, leading to entire instances having apt broken later
  • Apps with clear security issues (e.g. exposing a port through the firewall when it's not actually needed ...)
  • Apps that do not respect the spirit of YunoHost (e.g. glowing bear, where you need to manually install weechat (why??), or other apps where you're supposed to finish the installation yourself)
  • Apps editing nginx configuration manually
  • etc...

Yet, we should have a mechanism to be able to cap the level of these apps ... So this PR introduces a mechanism where :

  • it checks that the app exists in the YunoHost app's catalog
  • gets the corresponding url
  • check on the corresponding repo if there are any opened issues starting with [Low Quality] or [Broken]
  • if there are, it will return an error. Also we need to return a special error code in case of Broken such that the app gets flagged as level 0. (Gotta do a PR on package_check to handle this)

package_linter.py Outdated Show resolved Hide resolved
Co-Authored-By: tituspijean <tituspijean@outlook.com>
kay0u
kay0u approved these changes Dec 1, 2019
Copy link
Member

@kay0u kay0u left a comment

I think it's a good idea.

We could also imagine a package_check more robust, who could check apt packages installed before installing the app and after removing it, check if nginx (or any other system conf file) has been modified manually, but anyway, we can't handle all the imagination of packagers and a manual checking stays a good idea imo.

@Aeris1One
Copy link

@Aeris1One Aeris1One commented Apr 4, 2021

Would be useful in that case where the app is totally broken but the CI think it's ok : YunoHost-Apps/grav_ynh#65

Copy link

@Aeris1One Aeris1One left a comment

Code seems ok to me

@andretheolauret
Copy link

@andretheolauret andretheolauret commented Sep 5, 2021

Should be ok for merging

@Aeris1One
Copy link

@Aeris1One Aeris1One commented Dec 10, 2021

Smol bump ?

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.

None yet

5 participants