Draft impl#1
Conversation
Lenormju
left a comment
Lenormju
left a comment
There was a problem hiding this comment.
I have reviewed the PR. Most of the comments are notes or suggestions, there are very few blocking comments, and only 1 issue.
| from decimal import Decimal | ||
| import httpx | ||
| from app.config import PAYMENT_API_URL, PAYMENT_API_KEY |
There was a problem hiding this comment.
question(non-blocking): no ruff here ?
There was a problem hiding this comment.
ruff format is applied, and ruff check with default config (so not many rules).
What rules do we want for ruff ? Perhaps as-is for the current app, and all rules for the solution ?
There was a problem hiding this comment.
solution/app now pass with ruff check --select ALL --fix --ignore EM,TRY003
solution/test now pass with ruff check --select ALL --fix --ignore EM,TRY003,S,ANN,PLR2004 .
There was a problem hiding this comment.
I was expecting an empty line between the 3 categories of imports, and the API_URL to be sorted after the API_KEY, strange.
There was a problem hiding this comment.
The solution is not considered as a proper package by ruff, so not considered as "project code" but "external code".
|
|
||
| --- | ||
|
|
||
| ## 3. Existing tests: catalogue of problems |
There was a problem hiding this comment.
todo: @Lenormju review if you find more problems
Lenormju
left a comment
Lenormju
left a comment
There was a problem hiding this comment.
GitHub reviews suck! Sorry, I will have to spam you it seems 😢
| from decimal import Decimal | ||
| import httpx | ||
| from app.config import PAYMENT_API_URL, PAYMENT_API_KEY |
There was a problem hiding this comment.
I was expecting an empty line between the 3 categories of imports, and the API_URL to be sorted after the API_KEY, strange.
|
todo: add |
…che in CI in solution
…e in solution, add a comment about the db fixture, package the app, add the title to the abstract
There was a problem hiding this comment.
issue: faudrait tester sur Windows, car il y aura statistiquement au moins une personne sur Windows qui assistera au workshop. Mais j'ai pas de PC Windows sous la main, que du Linux. Toi tes VMs sont pas sous Windows ?
There was a problem hiding this comment.
Sous Windows, la partie Python fonctionne, mais j'ai pas Docker
2 participants
No description provided.