Arangodantic - Pydantic -> ArangoDB: plz Review

Installation und Anwendung von Datenbankschnittstellen wie SQLite, PostgreSQL, MariaDB/MySQL, der DB-API 2.0 und sonstigen Datenbanksystemen.
Antworten
Tholo
User
Beiträge: 177
Registriert: Sonntag 7. Januar 2018, 20:36

Hej!
Wiedermal hab ich mich an ein Projekt gemacht. ABER es kam wirklich mal zu einem PR auf GH.
Arangodantic Pull #20
Ich würde mich freuen, wenn ihr euch mal den Code anschauen würdet. Da ich auf Rückmeldung und Code Review gespannt bin.
Ich habe den bestehenden Code auf Pydantric V2 umgeschrieben und anstatt eine veraltete ArangoDB Client Version (welcher nur await und async vor die sync func vom normalen "Python ArangoDB Client" erweitert hatte) Asyncer als Modul genutz. Um im async eventloop Sync Funktionen aufzurufen. Das kommt von dem Author von FastApi. In meinen Tests läuft es gut. Kann aber in Sachen Profiling nicht genauers dazu sagen.

Ein paar Probleme hab ich schon mit dem Shylock gefunden. Das habe ich aber noch nicht durchdrungen. Da ich den Nutzen und damit die Funktion von Shylock noch nicht ganz kapiert habe.

Wie gesagt, ich würde mich freuen wenn ihr mal drüber schauen würdet und mir eure Anmerkungen dazu gebt.

Vielen Dank
__deets__
User
Beiträge: 14543
Registriert: Mittwoch 14. Oktober 2015, 14:29

Ich verstehe nicht so ganz, warum wir das anschauen sollen, wenn das doch ein PR bei einem Projekt ist? Entsprechend bin ich zumindest auch inhaltlich dazu gar nicht in der Lage, weil ich das Projekt nicht kenne.

Rein von der Form her: ich erwarte sowas als kleinteilige commits. Wo die einzelnen refactorings und Features getrennt aufeinander aufbauen. Das erleichtert üblicherweise das review. Wenn da WIP für work in progress steht, muss das erst recht extra, und ggf in einen eigenen PR. Generell ist es vorteilhaft, PRs klein zu halten, und lieber mehrere aufeinander aufzubauen, damit reviews einfacher und schneller sein können.
Antworten