-
Notifications
You must be signed in to change notification settings - Fork 211
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
Add --ignore to ignore user #546
Conversation
for #513 |
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.
Hi, few things:
- The option you added to the documentation is
--ignore
, not--ignore-user
- The if statement shouldn't be in the cloner, it should be in the downloader class. The cloner simply inherits from both of them
- If you could write tests for the relevant sets of integrations (the cloner, downloader, and archiver) that would be great. The BDFR needs to have every change tested
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.
Hey, thanks for fixing the documentation. This test doesn't pass, in fact it doesn't run at all. I more meant integration tests, although it's not bad to have this one as well.
b3c77e7
to
2dd446a
Compare
8a3c1c5
to
383c0f6
Compare
383c0f6
to
f670b34
Compare
No description provided.