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

Adding Bankverbindung and Erreichbarkeit COM objects #466

Merged
merged 26 commits into from
Aug 6, 2024

Conversation

hamidhajiparvaneh
Copy link
Contributor

No description provided.

Copy link
Collaborator

@hf-kklein hf-kklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich würde mir wünschen, dass

  • die Erreichbarkeiten irgendwie wie Zeiten oder Zeiträume typisiert sind
  • Bei den Bankverbindungen noch Beispiele stünden. In 99.0% der Fälle reicht doch ws. IBAN+BIC und der Rest ist nur für Steuerflüchtige in Richtung Seychellen relevant? Oder ich kenne mich mit dem Kontext an der Stelle nicht aus. Könnte man das vllt deutlicher machen?

Letztlich sollte es jemand beurteilen/reviewen, der/die die fachlichen Anforderungen kennt.

BO4E/COM/Erreichbarkeit.cs Outdated Show resolved Hide resolved
BO4E/COM/Erreichbarkeit.cs Show resolved Hide resolved
@hamidhajiparvaneh
Copy link
Contributor Author

hamidhajiparvaneh commented Aug 5, 2024

  • Bei den Bankverbindungen noch Beispiele stünden. In 99.0% der Fälle reicht doch ws. IBAN+BIC und der Rest ist nur für Steuerflüchtige in Richtung Seychellen relevant? Oder ich kenne mich mit dem Kontext an der Stelle nicht aus. Könnte man das vllt deutlicher machen?

Ich hatte gedacht, es ist besser, wenn wir das ganze Segment von MIG hier mappen kann.
In dieser Beispiel von PARTIN ist ja nur IBAN:Kontoeinhaber+BIC::::::Bankname benötigt.

FII+BK+DE00000000000000000000:Unternehmens GmbH+BICXXXXXXXX::::::Bankname' 

image

BO4E/BO/Zeitfenster.cs Outdated Show resolved Hide resolved
BO4E/BO/Zeitfenster.cs Outdated Show resolved Hide resolved
BO4E/BO/Zeitfenster.cs Outdated Show resolved Hide resolved
BO4E/BO/Zeitfenster.cs Outdated Show resolved Hide resolved
hamidhajiparvaneh and others added 6 commits August 6, 2024 09:54
Co-authored-by: konstantin <konstantin.klein@hochfrequenz.de>
Co-authored-by: konstantin <konstantin.klein@hochfrequenz.de>
Co-authored-by: konstantin <konstantin.klein@hochfrequenz.de>
@hf-kklein
Copy link
Collaborator

kannst du vor dem merge noch den openapi/json-schema generation neu ausführen?

# powershell.exe C:/github/BO4E-dotnet2/SchemaGenerator/generate-json-schemas.sh [jsonOutputDirectory] [openApiOutputDirectory]

das passiert leider noch nicht automatisch.

@hamidhajiparvaneh
Copy link
Contributor Author

kannst du vor dem merge noch den openapi/json-schema generation neu ausführen?

# powershell.exe C:/github/BO4E-dotnet2/SchemaGenerator/generate-json-schemas.sh [jsonOutputDirectory] [openApiOutputDirectory]

das passiert leider noch nicht automatisch.

Ich habe es ausgeführt, aber kein changes!

@hf-kklein
Copy link
Collaborator

War dein CWD der repo Root oder das SchemaGenerator Verzeichnis?

BO4E/COM/Bankverbindung.cs Outdated Show resolved Hide resolved
BO4E/COM/Bankverbindung.cs Show resolved Hide resolved
[JsonPropertyName("land")]
[ProtoMember(8)]
[JsonPropertyOrder(8)]
public Landescode? Land { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brauchen wir das Land? Das ist ja eigentlich mit den ersten beiden Zeichen der IBAN abgedeckt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BO4E/COM/Zeitfenster.cs Show resolved Hide resolved
@hf-fvesely
Copy link
Contributor

Oh sorry ich habe eure ganzen anderen Kommentare irgendwie nicht während meines Reviews gesehen, ich schau ncohmal was davon noch aktuell sit, moemnt

@hamidhajiparvaneh
Copy link
Contributor Author

War dein CWD der repo Root oder das SchemaGenerator Verzeichnis?

Beides habe ich probiert, aber keine Änderungen.

@hamidhajiparvaneh
Copy link
Contributor Author

War dein CWD der repo Root oder das SchemaGenerator Verzeichnis?

Ich könntees schaffen mit .ps1 file (in windows), habe ich es gepusht: 98e74fb

Copy link
Contributor

@JoschaMetze JoschaMetze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@hamidhajiparvaneh hamidhajiparvaneh merged commit 05abfd3 into main Aug 6, 2024
13 checks passed
@hamidhajiparvaneh hamidhajiparvaneh deleted the PARTIN_changes branch August 6, 2024 14:07
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

4 participants