Autor Thema: PMD-CPD Analyse nach Duplikaten, Codeverdopplungen, Copy & Pastes und so  (Gelesen 1821 mal)

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 22636
Zitat
Es gibt ja zur Zeit leider keinen Test/Dev Zweig im svn...
Das ist falsch, siehe svn list svn+ssh://svn.fhem.de/branches, und keiner kraeht danach, wenn jemand einen Neuen anlegt.
Das Problem ist: es ist eine Privatveranstaltung, keiner macht sich die Muehe es zu testen. Die Entwickler nicht (die wollen ihren eigenen Kram entwickeln), und die Benutzer nicht, die wollen ein stable branch mit den neuesten Features.

Der Hintergrund zu den Maintainer-Prinzip: es hat sich nach anfaenglichen Experimenten ohne eine Richtlinie als sinnvoll erwiesen, wenn jemand weiss, was in einem Modul "los ist". Sonst baut jemand ein Feature ein, was unter anderem Namen schon gibt, und unvertraeglich mit einem dritten ist. Wenn ein Benutzer ein Problem meldet, dann zuckt der urspruengliche Entwickler sein Schulter (er hat den Mist nicht eingebaut), und der Wohlwollende auch, der will/kann gar nicht Support leisten.
Es geht darum, einen Verantwortlichen fuer ein Modul zu haben. Wenn mehrere es schaffen, das unter sich zu organisieren, ist es mir egal, aber ich will einem Maintainer garantieren, dass er die Chance hat, die Uebersicht zu behalten.

Kein Regel ohne Ausnahme: wenn ein neuer Regel (z.Bsp. bei der Formatierung der Doku) in Developer kommuniziert, und nach Monaten nicht angewendet wird, dann wird es trotzdem durchgefuehrt. Ist aber selten, und war bisher nur auf die Doku bezogen.

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 596
  • HoBo: zwischen Weltherrschaft und Intensivstation
    • Experimenteller FHEM Fork
Wie wäre es mit einem OPT-OUT?

Wer keinen Janitor in seinem Code haben möchte, macht am Anfang eine Zeile

# FHEM-TAGS: NOJANITOR

(dann können wir uns auch noch andere Tags überlegen), und dann wird der Code in Ruhe gelassen.
Ist zwar IMHO kontraproduktiv, weil es u.U. globale Aufräumarbeiten wie min/max torpedieren kann und deswegen auch OPT-OUT und nicht OPT-IN. Wer sich dem explizit entgegenstellen will, soll das auch sagen.

Aber ich verstehe die Bedenken von Maintainern schon. Vermutlich ist die ganze Geschichte - wie so vieles - einfach nur Vertrauenssache.

"Ich habe ein Modul für das ich verantwortlich bin, da will ich nicht, dass mir das einer zerschiesst."

Dann gebe ich hiermit zu Protokoll, dass diese Sichtweise eines Maintainers richtig ist. Ich würde das genauso sehen. Wenn ich für etwas verantwortlich bin und eine drite Seite grätscht rein und verursacht eine Regression, dann drücke ich vielleicht einmal das Auge zu, beim zweiten Mal ist der OPT-OUT so schnell drin, so schnell kann keiner svn commit sagen. Bzw. ich klage bitterlich und verlange nach einem besseren Janitor.

Offline herrmannj

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 5638
Ich erlaube mir das Thema 'ein Modul - ein file' vs 'eigene libs' nochmals noch oben zu bringen. Bisher hat sich niemand gemeldet was idr entweder bedeutet 'kein Interesse' oder die Frage ist untergegangen.

Im ersteren Fall ist es ok. Von meiner Seite aus würde ich es begrüßen die Möglichkeit zu haben. In der Praxis findet es ja auch schon teilweise statt, ist aber imho nirgendwo sauber geregelt.
« Letzte Änderung: 12 April 2020, 12:59:55 von herrmannj »
smartVisu mit fronthem, einiges an HM, RFXTRX, Oregon, CUL, Homeeasy, ganz viele LED + Diverse

Offline CoolTux

  • Developer
  • Hero Member
  • ****
  • Beiträge: 25721
Ich wäre dafür zu versuchen einen modernen Programmierstil ein zu führen.
Klassen Unterteilung und Bibliotheken wo wir mehrfach vorkommenden Code auslagern.
Du musst nicht wissen wie es geht! Du musst nur wissen wo es steht, wie es geht.
Support me to buy new test hardware for development: https://paypal.me/pools/c/8gULisr9BT
My FHEM Git: https://git.cooltux.net/FHEM/
Mein Dokuwiki:
https://www.cooltux.net
Zustimmung Zustimmung x 1 Liste anzeigen

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 596
  • HoBo: zwischen Weltherrschaft und Intensivstation
    • Experimenteller FHEM Fork
Ich erlaube mir das Thema 'ein Modul - ein file' vs 'eigene libs' nochmals noch oben zu bringen.

Ich wollte da ursprünglich bitten etwas Nachforschung zu betreiben. Sprich: gibt es diese Regel wirklich irgendwo oder ist das nur so ein Mythos?
Du konntest die ja laut eigener Aussage auch nirgends finden.

Denn für gewöhnlich ist die Verwendung von mehr als einem package pro File zwar möglich, aber wird nicht als gute Praxis angesehen.
(siehe z.B. https://perlmaven.com/packages-modules-and-namespace-in-perl)

Und vom technischen her gibt es auch nicht viel zu diskutieren. Die ganze use/require Infrastruktur von Perl geht davon aus, dass man ein

use FHEM::Washlet::TOTO;

auch in einem @INC Verzeichnis unter FHEM/Washlet/TOTO.pm vorfindet.

Ich weiß, dass ich mir mit solchen Aussagen nicht viele Freunde mache, aber gegenwärtig weiß ich echt nicht wie man in einem Perl Projekt dieses filename <-> package name schlimmer machen könnte, als es gegenwärtig in FHEM praktiziert wird. Also selbst wenn ich bewusst wollte - weiß ich nicht wie. Und meine Fantasie ist nicht gering.

Offline Wzut

  • Developer
  • Hero Member
  • ****
  • Beiträge: 3621
Ich hätte auc noch was. Als ich das 38_BEOK Modul geschrieben habe brauchte ich Perl Code für das Broadlink Protokoll.
Was lag also näher als alles was ich brauchte mir aus 38_Broadlink zu "borgen" und im Kopf meines Moduls eine Zeile zu hinterlassen :
# Broadlink protocol parts are stolen from 38_Broadlink.pm :) , THX to daniel2311Wie bitte löst man sowas elegant ? Die betroffenen Subs in eine eigene Datei auslagern ? und wer ist dann für diese Datei vernatwortlich ?
bzw. der andere Maintainer muß dabei auch mitspielen und sein Modul auch anpassen.
Maintainer der Module: MAX, MPD, UbiquitiMP, UbiquitiOut, SIP, BEOK, readingsWatcher

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 596
  • HoBo: zwischen Weltherrschaft und Intensivstation
    • Experimenteller FHEM Fork
Ich hätte auc noch was. Als ich das 38_BEOK Modul geschrieben habe brauchte ich Perl Code für das Broadlink Protokoll.
Was lag also näher als alles was ich brauchte mir aus 38_Broadlink zu "borgen" und im Kopf meines Moduls eine Zeile zu hinterlassen :
# Broadlink protocol parts are stolen from 38_Broadlink.pm :) , THX to daniel2311Wie bitte löst man sowas elegant ? Die betroffenen Subs in eine eigene Datei auslagern ? und wer ist dann für diese Datei vernatwortlich ?
bzw. der andere Maintainer muß dabei auch mitspielen und sein Modul auch anpassen.

Elegant löst man sowas so, dass es ein

FHEM::Broadlink

unter FHEM/Broadlink.pm gibt

und Du machst dann in Deinem Modul ein

use FHEM::Broadlink qw(magic_sub);

Falls ich das recht verstanden habe, diskutieren wir gerade ob wir künftig elegant wollen.
Gefällt mir Gefällt mir x 2 Liste anzeigen

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 22636
Zitat
Sprich: gibt es diese Regel wirklich irgendwo oder ist das nur so ein Mythos?

Wenn man ein Schreibzugriff aus SVN bekommt, kriegt man die:
Zitat
Standard Belehrung:
========
- nur die eigenen Module modifizieren, sonst dem Maintainer Patches schicken.
- nach FHEM kommen nur die Module, die dokumentiert und im Forum betreut sind
  (siehe MAINTAINER.txt), sonst nach contrib.
- falls das Modul nach FHEM kommt, dann bitte das betroffene Forum (wg.
  Support) und Developer (wg. allgemeine API-Aenderungen) abonnieren.
- vor dem Einchecken alles (auch Doku, mit contrib/commandref_join.pl &
  Browser) testen, und die letzten Aenderungen mit "svn diff FHEM/MyModul.pm"
  pruefen.
- neue Verzeichnisse duerfen nur nach Ruecksprache mit mir angelegt werden, das
  Gleiche gilt fuer das Einchecken von fremden Dateien wie Bibliotheken, usw.
========

Der letzte Punkt is reingekommen, weil ich CPAN nicht doppeln, oder Bibliotheken auf Lizenz pruefen will. Weiterhin ist fhemupdate.pl absichtlich starr, sprich man kann zwar ein Verzeichnis in SVN anlegen, aber das kriegen die Benutzer noch lange nicht per update.

Ich bin offen diesen Regel aufzuweichen, wenn dabei mein Bedenken beruecksichtigt wird.

Offline herrmannj

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 5638
absolut! Eventuell kommen ja auch noch Beiträge und Gedanken weiterer devs dazu.

Verstehe ich richtig:

- "fremd" bezieht sich auf Code "von anderen", hauptsächlich wegen möglicher Risiken mit Bezug auf die Lizenz. Aber auch um kein CPAN 2 aufzumachen.
- "eigene" Libs sind von den bestehen Regeln gedeckt wenn die im /FHEM Verzeichnis landen. (in /FHEM aber eben flat, keine sub dirs)

Über ein "nicht flat" (da geht's ja in die Richtung namespaces) könnte man reden, die Rules müssten festgelegt werden.

So richtig wiedergegeben ?


smartVisu mit fronthem, einiges an HM, RFXTRX, Oregon, CUL, Homeeasy, ganz viele LED + Diverse

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 596
  • HoBo: zwischen Weltherrschaft und Intensivstation
    • Experimenteller FHEM Fork
Wenn man ein Schreibzugriff aus SVN bekommt, kriegt man die:

Ok. Nichts darin verbietet, dass man zur Implementierung von Funktionalität X nicht mehr als 1 Modul verwenden darf.
Ich denke, damit wäre dieses "self-contained" erstmal abgehakt. Problematisch ist nur das kein Verzeichnis anlegen...

Ich habe da jetzt knapp zwei Stunden darüber sinniert und muss kleinlaut zugeben, dass mir eine richtig gute FHEM-kompatible Lösung nicht einfällt. Oder ich formuliere es mal um:

Wenn man sich HoBo anschaut, dann sieht man schon was ich mir als Ideale Lösung vorstelle:

Im root -Verzeichnis sind zwei Verzeichnisse lib und t dazugekommen. Ersteres soll Perl Module nach der regulären Nomenklatur (X::Y::Z = X/Y/Z.pm) enthalten, letzteres die zugehörigen Testfiles. Diese Vorgehensweise würde ich auch bei FHEM sehr stark ans Herz legen.

In lib tatsächlich nur Perl Module und nur nach o.g. Nomenklatur. Kein JavaScript, XML, .GZ und sonstiges Gedöns. (sowas gehört nach shared, data, resources oder so)

FHEM/lib werde ich entsprechend knicken, sobald ich die Gelegenheit dazu bekomme.

Mein Ziel ist, dass es mit der Zeit im FHEM Unterverzeichnis nur noch xx_Modulname.pm  main::-Interfaces (Hülsen sozusagen) gibt, die eigentlich nur ein use FHEM::MeinModul::Funktion qw(MeinModul_Init MeinModul_Define ...) machen, die Hauptlogik lebt aber unter lib/* und wird getestet von t/*



Tja ... aber das kann man alles machen, wenn man ne Installationsbasis von < 10 Diehards hat.  8)

Ich habe es mir auch noch einfacher gemacht und contrib erstmal gekillt. Folglich keine Ahnung wie man contrib vs. non-contrib handhaben könnte. Vermutlich müsste man ./lib & ./t nur für Modulautoren die ein FHEM/xx_Frontend.pm haben freigeben und für contrib Code sperren, womit man aber ggf. Contrib torpediert. Ist ein wenig Zweiklassengesellschaft, wofür ich keine guten Konzepte habe. Ich kann das nicht einmal mit staging/production abbilden.

Module unter ./lib anlegen würde ich nicht von "nach Rücksprache mit Nadelöhr" abhängig machen, sondern von "gemäß Nomenklatur/Hierarchie und announcement in <FHEM Forum/Development/Namespace>", aber der Modulautor macht einfach. Ähnlich wie PAUSE bei CPAN.



Auf jeden Fall, sollte das Ziel sein, dass ein Modulautor, der heute - Beispiel aus der Rippe geschnitten -

70_ONKYO_AVR.pm
71_ONKYO_AVR_ZONE.pm
ONKYOdb.pm


maintained, künftig folgendes nicht nur machen darf, sondern als "best practice" präsentiert bekommt:

* es gibt ein 70_ONKYO_AVR.pm modul, das ist aber nur ein <10-20 Zeilen Skelett

package main;

use Multimedia::AVR::ONKYO  qw(Init Define ...);

<ggf notify defs>

1;


* unter lib gibt es dann

lib/Multimedia/AVR/ONKYO.pm
lib/Multimedia/AVR/ONKYO/Zone.pm
lib/Multimedia/AVR/ONKYO/Db.pm


welche die eigentliche Codelogik in gekapselten Namespaces bereitstellen.  Ob man dann 71_ONKYO_AVR_ZONE.pm knickt oder nicht sei dahingestellt.



Gemäß FHEM Kulturgepflogenheiten ist das alles aber kein Muss. Wenn ein Autor/Maintainer alles im xx_Modulname.pm im main:: Namespace belassen will, belässt er es einfach dabei. Aber wer sein Modul auf ein höheres Level hieven will, kann das dann ja machen.

Beide Arten von Modulen können nebeneinander koexistieren. Mit der Zeit kommt vermutlich Darwin zu Wort.



Sprung in die ferne Zukunft, der Mars ist besiedelt, FHEM bei Version 97.1

Die Enkel des ONKYO Maintainers haben das nun auch gemacht, ebenso haben das die Enkel der YAMAHA_AVR und PONEER_AVR Modulmaintainer gemacht.
Nun kommen die Enkel von RichardCZ und behaupten man könne alles viel besser machen und knicken den verbleibenden Rest von xx_Frontend Modulen, bauen stattdessen ein dynamisches Laden von Modulen mit rekursivem Absiteg in die ./lib/* Hierarchie und machen z.B. ein abstraktes Modul

Multimedia/AVR.pm

was der werte User von FHEM 97.1 als AVR Device definiert und es ist ihm komplett wurst welches Gerät von welchem AVR Hersteller er hat.
Gefällt mir Gefällt mir x 1 Liste anzeigen

Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 22636
Zitat
So richtig wiedergegeben ?
Ja.

Ich haette gerne eine Loesung, was die Regeln per Code sicherstellt, ich will nicht taeglich schauen, dass jemand alles was fuer DateTime::Event::Sunrise benoetigt wird, eingecheckt hat und ausliefert.
Einchecken alleine ist nicht so schlimm: wenn es nicht ausgeliefert wurde, kann man es leicht entfernen.
Update kann keine Dateien (mehr) loeschen, nur verschieben, und selbst dabei habe ich ein schlechtes Bauchgefuehl.

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 596
  • HoBo: zwischen Weltherrschaft und Intensivstation
    • Experimenteller FHEM Fork
Ja.

Ich haette gerne eine Loesung, was die Regeln per Code sicherstellt, ich will nicht taeglich schauen, dass jemand alles was fuer DateTime::Event::Sunrise benoetigt wird, eingecheckt hat und ausliefert.

Ich hätte im Angebot:

* geradezu faschistische SVN pre-commit, post-commit und pre-vomit  ;) hooks. Aber ich bezweifle, dass die FHEM kulturkompatibel sind
* Die Möglichkeit für CI/CD Pipelines - https://gl.petatech.eu/root/HomeBot/pipelines (selber für HoBo noch nicht eingerichtet, aber das Werkzeug ist da)
https://docs.gitlab.com/ee/ci/introduction/

Sollte FHEM e.V. doch noch über eine self-hosted GitLab Infrastruktur nachdenken: https://www.turnkeylinux.org/gitlab
Läuft bei mir unter Proxmox, war in 30 Minuten eingerichtet. Updates seit Version 11 problemstlos.

Jaja - ich weiß - gestern erst noch von CVS auf SVN migriert und jetzt wieder so neumodisches Zeug.  ;)

Um auf das DateTime::Event::Sunrise zurückzukommen:

da gibt es natürlich ein

t/DateTime/Event/Sunrise/00_smoke.t

Das kann ein einfaches use auf das Modul machen.
und wenn das in so einer CI pipeline auf die Schnauze fliegt, kann man ja dem Modulautor um 2 Uhr früh via SMS Bescheid geben.


Zitat
Update kann keine Dateien (mehr) loeschen, nur verschieben, und selbst dabei habe ich ein schlechtes Bauchgefuehl.

Tja. Rollback is king. Statt:

my $destdir="/var/www/html/fhem.de";

eben

my $destdir="/var/www/html/fhem.de/active";

und unter /var/www/html/fhem.de sieht es eben aus wie

active -> 6.0
6.0
6.1
6.2

wenn einer putt ist, schaltet man zurück.

Offline DS_Starter

  • Developer
  • Hero Member
  • ****
  • Beiträge: 5760
Ich brauche mal kurz einen Anstoß.
Wie kann man das PMD-CPD Analysetool aufrufen ?

Ein paar Stellen in meinen Modulen die ich diesbezüglich schon einige Zeit verbessern will kenne ich auch ohne das Ding,
aber nicht alle.

Grüße,
Heiko
ESXi 6.5 @NUC6i5SYH mit FHEM auf Debian 10, DbLog/DbRep mit MariaDB auf Synology 415+
Maintainer: SSCam, SSChatBot, SSCal, DbLog/DbRep, Log2Syslog, SMAPortal, Watches, Dashboard
Kaffeekasse: https://www.paypal.me/HMaaz
Contrib: https://svn.fhem.de/trac/browser/trunk/fhem/contrib/DS_Starter

Offline RichardCZ

  • Tester
  • Sr. Member
  • ****
  • Beiträge: 596
  • HoBo: zwischen Weltherrschaft und Intensivstation
    • Experimenteller FHEM Fork
Ich brauche mal kurz einen Anstoß.
Wie kann man das PMD-CPD Analysetool aufrufen ?

Ein paar Stellen in meinen Modulen die ich diesbezüglich schon einige Zeit verbessern will kenne ich auch ohne das Ding,
aber nicht alle.

z.B.

/usr/bin/pmd-cpd --minimum-tokens 100 --files FHEM/ --language perl


im FHEM root-Verzeichnis. Dann geht er rekursiv alle "Module" in FHEM durch.

Offline DS_Starter

  • Developer
  • Hero Member
  • ****
  • Beiträge: 5760
Danke für die Info Richard.
ESXi 6.5 @NUC6i5SYH mit FHEM auf Debian 10, DbLog/DbRep mit MariaDB auf Synology 415+
Maintainer: SSCam, SSChatBot, SSCal, DbLog/DbRep, Log2Syslog, SMAPortal, Watches, Dashboard
Kaffeekasse: https://www.paypal.me/HMaaz
Contrib: https://svn.fhem.de/trac/browser/trunk/fhem/contrib/DS_Starter

 

decade-submarginal