5minds / javascript-guidelines

Our customized eslint config inspired by airbnb
0 stars 0 forks source link

Vereinheitlichung mit TSLint #10

Open ElRaptorus opened 6 years ago

ElRaptorus commented 6 years ago

Es gibt derzeit 2 TSLint Regeln, bei denen man überlegen könnte, ob man sie auch auf ESLint überträgt:

  1. Max. Zeilenlänge: Bei ESLint beträgt diese 120, bei TSLint 150. Mir fällt gerade kein Grund ein, nicht auch bei ESLint 150 Zeichen pro Zeile zu erlauben.

  2. ESLint schreibt derzeit vor, Methoden erst zu definieren, bevor man sie verwendet. Das kann mMn ziemlich unglücklich werden, da sich das dem normalen Lesefluss ("von oben nach unten") entgegen stellt, was besonders bei komplexeren Integrationstests schnell unvorteilhaft werden kann.

Beispiel: Wir haben einen Test in der Process Engine, der die korrekte Ausführung eines TerminateEndEvents testen soll. Eine Verifizierung ist allerdings nur mit Datenbankzugriffen nach der Testausführung möglich, da über das reine verfügbare Prozessergebnis nicht sichtbar ist, ob die Aktivitäten eines Prozesses korrekt beendet - bzw. garnicht erst gestartet wurden. D.h. wir benötigen noch zusätzliche Funktionen, die den Datenbankzugriff und zugehörige Assertions steuern.

In Kurzfassung sieht der Test folgendermaßen aus:

  it('should successfully terminate a process upon reaching a TerminateEndEvent.', async () => {
    // Run test and perform assertions afterwards
  });

  async function assertActiveNodeInstancesWereTerminated(processInstanceId) {
     // Do stuff
  }

  async function assertPendingNodesWereNotCreated(processInstanceId) {
     // Do stuff
  }

  async function queryNodeInstanceByKey(processInstanceId, instanceConfig) {
     // Do stuff
  }

Nach aktueller ESLint Konvention, müssten die unten stehenden Funktionen vor sämtlichen it Definitionen und auch in umgekehrter Anwendungsreihenfolge stehen. Das ist unvorteilhaft, da dies, wie oben erwähnt, gegen den natürlichen Lesefluss gehen würde und mich beim Betrachten der Tests auch in erster Linie die it Blöcke interessieren, nicht unbedingt die Hilfsmethoden die ggf. durch diesen Test ausgeführt werden.

Besonders bei der Entwicklung der Evita API von Huf, bei der die Integrationstests auch häufig viele it cases und mehrere Hilfsmethoden umfassten, hatte uns diese Vorschrift häufig massiv behindert.

Daher denke ich, wir sollten diese Regel streichen.

LeoTT commented 6 years ago

Gibt noch ein paar andere Unterschiede, die ich vor Ewigkeiten mal festgehalten hatte https://github.com/5minds/JS.Foundation/issues/109

ElRaptorus commented 6 years ago

@LeoTT Nice!