-
Notifications
You must be signed in to change notification settings - Fork 24
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
🔨 fix blackWhiteList feature #40
Conversation
@Whisper40 peux tu mettre le retour de tes tests |
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.
While it works/could work, I am not seeing everything that can prove it: No tests.
Please add a unit test, covering all 3 cases mentioned here (possibly more with fuzzing).
Sans ce changement les nouveaux projects ne sont jamais créés si au moins un projet est présent dans le blacklist et que la fonctionnalité whitelist n'est pas activée
1688468
to
37af7d5
Compare
99e563c
to
65bd6be
Compare
65bd6be
to
4e0e4fb
Compare
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.
I didn't test the change. But at first sight, it looks okay.
I will approve, but I would like to see a follow up commit for two reasons:
- The tests are hard to read or lie a bit.
- Plenty of nits.
fb308fd
to
908d722
Compare
vu que nous avons adapter la fonction sur le précédent commit pour qu'il soit mieux testable, nous modifions ici les tests unitaires de la fonctions GenerateProject
908d722
to
0073ab6
Compare
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.
not tested but lgtm
Sans ce changement les nouveaux projects ne sont jamais créés si au moins un projet est présent dans le blacklist et que la fonctionnalité whitelist n'est pas activée