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

Extend type-annotations for datalad/support/path.py #7336

Merged
merged 2 commits into from
Mar 20, 2023

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Mar 16, 2023

Part of #6884.

@jwodder jwodder added semver-internal Changes only affect the internal API CHANGELOG-missing When a PR's description does not contain a changelog item, yet. and removed CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Mar 16, 2023
@github-actions github-actions bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Mar 16, 2023
# if no exception raised it was not the reason, raise original
raise
except:
except Exception:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha -- so even logic I added in 0.10.3~18^2 (53cfc2d) was wrong since I was re-raising and then right away going into except for it... odd. This fixes that now I think. BUT I would keep it

Suggested change
except Exception:
except:

or at least

Suggested change
except Exception:
except BaseException:

if possible to retain original effect from getcwd crashing one way or another.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bare except:/except BaseException: will also catch KeyboardInterrupt and other things that normally shouldn't be caught.

@yarikoptic yarikoptic merged commit 552a4a8 into datalad:maint Mar 20, 2023
@yarikoptic-gitmate
Copy link
Collaborator

PR released in 0.18.3

@jwodder jwodder deleted the type-path branch May 12, 2023 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-internal Changes only affect the internal API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants