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

Made a single error message not stall the repo finding process #11

Merged
merged 1 commit into from Nov 30, 2021

Conversation

douweschulte
Copy link
Contributor

Love the idea of this project. But on my machine it crashed on a single repo, so I refactored it so that it continues processing after encountering a erroneous repo. With this changes it works perfectly on my machine, it shows the error and picks up all other repos. It also picks up the repo that gave rise to the error in some way which I do not really understand but will look into.

The git changes look a bit all over the place, but essentially I moved the code after match open_repo into the match statement in the function find_repos.

I see you do not have any contributing guidelines or license in the repo but I assume you appreciate PRs and I allow my changes to be integrated in the code and licensed under any open source license.

@douweschulte
Copy link
Contributor Author

Update: the repo that gave rise to the issue indeed was broken. And it did not pick it up anyways it did pick up another repo with a very similar name.
So the current state of the code (without this PR) works. It is up to you if you want a fail early or fail late system.

@hakoerber
Copy link
Owner

Hello!

Very cool, thank you for the contribution! I think having the find command report an error and continuing is preferable to failing completely, so this is nice.

I see you do not have any contributing guidelines

Yes, I just opened #12 to remedy this. License is GPLv3 (see here, I just added the license as LICENSE as well).

If you don't mind, I'd add you to a CONTRIBUTOR file (with name and email). Is that ok for you?

@hakoerber hakoerber merged commit 115eb7c into hakoerber:master Nov 30, 2021
@douweschulte
Copy link
Contributor Author

Great, I am okay with being added to the contributor file.

@hakoerber
Copy link
Owner

Done!

If you have any further ideas for improvements, let me know!

@hakoerber
Copy link
Owner

See

def test_repos_find_with_invalid_repo():
for an e2e test to prevent regressions.

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.

None yet

2 participants