lk3de / impftermin-checker

Search Impftermine and Vermittlungscodes in Germany automatically
GNU General Public License v3.0
21 stars 9 forks source link

added light version for bot #4

Closed Selfie21 closed 3 years ago

Selfie21 commented 3 years ago

Hii! Vielen Dank für den Bot hatte auch schon angefangen einen zu schreiben, bis ich deinen gesehen hab! Ich hab ihn etwas umgeschrieben und ein paar Timings verändert. Außerdem wird ne Exception geworfen, hab alles mal auf nen anderen Branch gemacht.

lk3de commented 3 years ago

Sehr cool, vielen Dank dir! 🎉 Das Refactoring in Klassen ist super und bei mir längst überfällig. Auch der Zufallswert bei der Wartezeit gefällt mir - hast du da schon Erfahrungen sammeln können, ob das etwas bringt in Bezug auf die sporadisch auftretenden HTTP 429 Fehler (siehe #2)?

Ich würde deine Änderungen vorerst als Pull Request stehen lassen, da ich ungern die E-Mail Notifications im Masterbranch komplett rauswerfen will. Sobald ich Zeit dafür finde, übernehme ich gerne das Refactoring + deine Bugfixes in den master, wenn das für dich okay ist 🙂 Da ich inzwischen schon alle Impftermine beisammen habe, hat das allerdings bei mir nicht die höchste Priorität. Ich kann aber gern alles andere mergen, solange keine existierende Funktionalität verloren geht. 😊

MeFisto94 commented 3 years ago

Wenn ich das richtig sehe, ändert dieser PR den Selenium Driver von Firefox auf Chrome. Wäre eventuell nett, das konfigurierbar zu haben oder gar das zu nehmen, was gerade für den Nutzer verfügbar ist, sonst wäre es quasi ein Breaking Change.

Außerdem +1 für die E-Mail, ich war zu faul Telegram dafür einzurichten, E-Mail war da einfach und quasi äquivalent funktional.

Selfie21 commented 3 years ago

Oh! Mein Fehler ich wollte eigentlich einen seperaten light branch nicht auf master mergen tsss! Werde das heute abend mal ausbessern. Zu dem Bug mit J&J Impftermin: ich habe da einfach das 2. Datum gelöscht zum checken, ich hab in meinen Fällen immer nur das erste gebraucht.

lk3de commented 3 years ago

Wenn ich das richtig sehe, ändert dieser PR den Selenium Driver von Firefox auf Chrome. Wäre eventuell nett, das konfigurierbar zu haben oder gar das zu nehmen, was gerade für den Nutzer verfügbar ist, sonst wäre es quasi ein Breaking Change.

Habe dafür mal #8 angelegt.