-
Notifications
You must be signed in to change notification settings - Fork 12
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
Implemented topics replies #103
Conversation
Risolve #73 |
Sono lontano da internet e senza pc per giorni, scusa. |
Provo a guardarci io oggi o domani
Il gio 10 ago 2023, 17:35 Stefano Volpe ***@***.***> ha
scritto:
… Sono lontano da internet e senza pc per giorni, scusa.
—
Reply to this email directly, view it on GitHub
<#103 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APEGQFRKP4X2KKKKPPP7E23XUT5TRANCNFSM6AAAAAA3KGA44Q>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
tgbotapi "github.com/go-telegram-bot-api/telegram-bot-api" | ||
tgbotapi "github.com/musianisamuele/telegram-bot-api" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Secondo me sarebbe meglio provare a tenere il "bot ufficiale" invece che una fork personale, riusciresti a creare pr sull'altro bot? Credo poi sia una feature che possa servire a molta gente.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ci sono almeno go-telegram-bot-api/telegram-bot-api#663 e go-telegram-bot-api/telegram-bot-api#665 che sono PR rilevanti per aggiungere supporto ai topic, ma non sono state mergiate e sono aperte da giugno. Quando fanno il merge si può ritornare alla libreria ufficiale comunque.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok ha senso, lo provo nattimo poi mergio se sembra tutto apposto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magari si può generalizzare la cosa che hai commentato sotto, ha senso.
if update.Message.IsTopicMessage { | ||
msg = tgbotapi.NewThreadMessage(update.Message.Chat.ID, | ||
update.Message.MessageThreadID, newCommand.Text) | ||
} else { | ||
msg = tgbotapi.NewMessage(update.Message.Chat.ID, newCommand.Text) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
si potrebbe fare na funzione incaricata di creare iil messaggio, e gestisca tutti i casi possibili (per ora topic o non topic), dato che è ripetuto anche sopra, però non credo sia molto importante.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Si ci sta, se è generalizzabile con una funzione che non prende parametri troppo assurdi assolutamente da fare.
Come mai la PR è stata mergiata con un force-push? Oltretutto non mi sembra che le richieste siano state soddisfatte |
force push per risolvere un conflitto. l'altra del refactor funzione non mi sembrava importante, tanto era comunque localizzata la cosa. |
Ok |
No description provided.