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

feature/automatische verbuchung von steuern #65

Conversation

dippeal
Copy link
Member

@dippeal dippeal commented Oct 31, 2023

In diesem PR sind die Steuer Erweiterungen von VinRud jverein#61 und Acrux-dev Acrux-dev@16115e7 enthalten.

Wegen der DB Updates ist dieses PR abhängig von #58 und sollte anschließend implementiert werden. (PR #58 = Update0419 | PR #64 = Update0420)

VinRud and others added 9 commits April 10, 2020 19:17
…teuer gebucht werden soll, kann jetzt angegeben werden
- zusätzlich gibt es nun eine DependecyId in der Buchung, welche abhängige Buchungen in einer Splitbuchung anzeigt (über die DB Id ist dies nicht möglich, weil der erst generiert wird, wenn alles gespeichert wird)
- Verwendungszweck ohne Bachtung der Groß/Kleinschreibung
- Ignorieren zusaetzliche Felder von Bankseite am Verwendungszweck
(IBAN)
- Stichtag im Abrechnungslauf anstatt Datum des Mitgliedskontos zur
Zeitraumselektion benutzen
…von-steuern' into feature/automatische-verbuchung-von-steuern
@willuhn
Copy link
Member

willuhn commented Nov 1, 2023

Mir persönlich sind das echt zu viele Änderungen auf einmal. Ich kann hier nicht wirklich erkennen, was die alle bewirken. Kleinere PRs lassen sich deutlich besser reviewen und können dadurch auch einfacher in den Master-Branch übernommen werden.

@dippeal
Copy link
Member Author

dippeal commented Nov 1, 2023

Kann ich verstehen. Das Problem bei Neuerungen/features ist aber oftmals, dass viel neuer Code dazukommt. Schließlich soll der Code auch keine Konflikte auslösen und lauffähig sein. Wie wäre dein Lösungsansatz hieraus kleinere PR zu erstellen?

@willuhn
Copy link
Member

willuhn commented Nov 1, 2023

Kleinere PR: Ein PR sollte nur genau ein Feature enthalten. Laut den Commit-Kommentaren (siehe oben) sind hier aber eine ganze Reihe von Änderungen zusammengefasst:

grafik

@dippeal
Copy link
Member Author

dippeal commented Nov 1, 2023

Wenn gewünscht trenne ich den Branch von VinRud und Acrux-dev in zwei separate PR. Die commits werden aber nicht kleiner. Weiterhin werden dann zwei Update Dateien (Update0420 und Update0421) benötigt, welche in Abhängigkeit zueinander stehen.
Wie gesagt, meine Intention ist, lauffähige PR zu bauen, so dass keine Konflikte entstehen.

@willuhn
Copy link
Member

willuhn commented Nov 1, 2023

Lauffähige PRs auf jeden Fall.

@NicoB77 Kannst du hier helfen? Ich kann den PR nicht überblicken - da stecke ich nicht tief genug im Code von JVerein drin.

@NicoB77
Copy link

NicoB77 commented Nov 2, 2023

Auf den ersten Blick haben die beiden Branches keine Abhängigkeiten, daher wäre es gut, den PR aufzuteilen.

Wegen der Datenbankupdates würde ich jeden PR so anlegen, dass er direkt gemerged werden kann (d.h. immer mit der nächsten Nummer, aktuell Update0419) und erst als letzten Schritt nach Approval gegebenenfalls den Konflikt durch Umbenennen der Update-Datei lösen. Dann muss keine Reihenfolge eingehalten werden, und es kann nicht versehentlich ein PR mit fehlerhaftem Datenbankupdate gemerged werden.

Spätestens nach dem Merge sollte auch das Handbuch angepasst werden.

Ich habe frühestens am Wochenende Zeit, mir das anzusehen.

@dippeal
Copy link
Member Author

dippeal commented Nov 2, 2023

Verstehe. Dann mache ich das mal.

@dippeal dippeal closed this Nov 2, 2023
@dippeal dippeal deleted the feature/automatische-verbuchung-von-steuern branch November 2, 2023 09:55
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