-
Notifications
You must be signed in to change notification settings - Fork 102
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 'hide-invalid-repositories' option #266
base: master
Are you sure you want to change the base?
Conversation
Hey @jonashaag, could you please consider merging this PR or giving your opnion on it? Many thanks! |
I’m sorry, I think I didn’t respond here because the tests fail and I wanted to wait for you to fix them before reviewing the PR |
Review feedback: please add the new option at the end of the keyword arguments. Also I think it’s more obvious to only look at the hide flag where the invalid repos are rendered, not where the list of invalid repos is constructed. |
Hey again, thank you for your time. I believe all is working this time! |
Thanks a lot, please move the new setting to very end so as to make the changes backwards compatible. Also what do you think about adding a "N invalid repositories hidden" to the repo list page? Do you think that could be helpful? Or maybe we can just log a warning to the terminal. Just a way for people to discover that some stuff has been hidden. Not sure what's the best option here. |
I could move it at the very end but the
Yes some stuff has been hidden but not by default, that's the user's will. I agree that both a normal logging as well as a small message to the repo list page could be useful. I'm on it :) |
I just tried this, but when I start |
The added option won't change this behavior. You can't pass an invalid Git repository to |
But how do you pass a folder on the command line? |
Using the standalone application ( By wrapping Klaus within |
I just investigated and found what causes that. When wrapping using One way around is to disable this git-type check in the argument parser but that's not the goal of this PR. :) EDIT: type check |
But then the command line flag does not make a difference, does it? |
I think it would be fine to have it disable the type check. |
This commit disables the type check. The command line flag doesn't make a difference with this previous type check as the repositories couldn't go "through" the type checker. Now it is disabled you can pass invalid repositories that will (or will not using this new option) get rendered. |
Sorry, maybe wasn't clear enough – we can change behaviour only when |
I don't think we can change behavior only when the option is given, that's all or nothing as the type check is done by the argument parser. If we really want to keep the Git type check, either we do it later or we keep this option as being useless when using the Python executable directly. |
This reverts commit b3b7088.
Yes, let's do the check later, for example in |
We could move the check in
To be honest, I can't really understand the type check and why it is done. |
Hey @jonashaag, have you had some time to think about those type checks? :) |
Hey.
As I don't mind if Klaus find some invalid repositories, I added an option to hide those.
During the app building, when it formats the {in,}valid repositories, it checks for this newly created option and dismiss the invalid ones.
I hope that's enough for you!
Thanks for this super software :)