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

Use library #117

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Use library #117

merged 1 commit into from
Nov 14, 2023

Conversation

VaiTon
Copy link
Member

@VaiTon VaiTon commented Oct 4, 2023

No description provided.

@VaiTon VaiTon self-assigned this Oct 4, 2023
@foxyseta
Copy link
Member

Scusa devi ancora lavorarci su questa vero? Giusto per essere sicuro che tu non stessi aspettando noi.

@VaiTon VaiTon marked this pull request as ready for review October 23, 2023 15:05
@VaiTon
Copy link
Member Author

VaiTon commented Oct 23, 2023

@foxyseta in realtà deve essere testata e basta mi sa

@foxyseta
Copy link
Member

Bisogna anche far passare i test giusto? Che al momento falliscono

@foxyseta
Copy link
Member

Se ti sccocia posso testalro anche io in futuro. Però abbiamo diversi TODO nel codice ancora giusto?

@VaiTon
Copy link
Member Author

VaiTon commented Oct 25, 2023

Yes, in realtà anche per questo era una draft. Il fatto è che uno di questi richiede un po' di modifiche nella libreria e l'altro just un po' di tempo

@foxyseta
Copy link
Member

Ah scusa allora lo lascio in draft. Era giusto per assicurarmi di non dover fare lavoro io che mi ero perso per strada. Se c'è da testare preferisco farlo in una volta sola alla fine.

@foxyseta foxyseta marked this pull request as draft October 25, 2023 09:50
@foxyseta
Copy link
Member

foxyseta commented Nov 1, 2023

Nuovi test sono stati aggiunti con #120 quindi dopo aver corretto i conflitti suggerisco di fare un rebase.

@VaiTon VaiTon force-pushed the use_library branch 5 times, most recently from 3c3aaff to d6f6551 Compare November 4, 2023 15:57
@foxyseta foxyseta linked an issue Nov 7, 2023 that may be closed by this pull request
2 tasks
@VaiTon VaiTon force-pushed the use_library branch 5 times, most recently from 13c66e5 to 000394a Compare November 10, 2023 22:37
@VaiTon VaiTon marked this pull request as ready for review November 10, 2023 22:43
Copy link
Contributor

@lucat1 lucat1 left a comment

Choose a reason for hiding this comment

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

Ho dato una letta molto veloce e non vedo particolari problemi.
Forse vogliamo comunque aspettare una review di @foxyseta , però se per me funziona va bene

Copy link
Member

@foxyseta foxyseta left a comment

Choose a reason for hiding this comment

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

Sì tutto bene. Dopo l'unione in main notifico del cambiamento chi sta lavorando su issue impattate. @VaiTon accetta pure quando vuoi. La tua chiave ssh è già presente sulla macchina di Inormabot?

Grazie per aver dato anche una pulita generale.

Pensavo di aprire una issue per dare anche una pulita alla gestione degli errori (al momento abbiamo funzioni che restituiscono un messaggio che gestiscono gli errori internamente ciascuna a modo suo, in moto tutt'altro che uniforme e centralizzato).

@VaiTon
Copy link
Member Author

VaiTon commented Nov 14, 2023

La tua chiave ssh è già presente sulla macchina di Informabot?

Non mi sembra sai

Pensavo di aprire una issue per dare anche una pulita alla gestione degli errori

Assolutamente d'accordo

@VaiTon VaiTon merged commit d4436c7 into main Nov 14, 2023
4 checks passed
@VaiTon VaiTon deleted the use_library branch November 14, 2023 20:42
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.

Utilizzare una libreria condivisa per l'accesso alle API di Unibo
3 participants