Fix some type checking errors after removing type hint ignores #567
Conversation
|
Not sure how to add reviewers (first time here), @ahal could you please review? |
src/taskgraph/main.py
Outdated
| @@ -862,7 +864,10 @@ def init_taskgraph(options): | |||
| context = {"project_name": root.name, "taskgraph_version": taskgraph.__version__} | |||
|
|
|||
| try: | |||
| repo_url = repo.get_url(remote=repo.remote_name) # type: ignore | |||
| if isinstance(repo.remote_name, str): | |||
There was a problem hiding this comment.
This check looks like it ought to be unnecessary. remote_name appears to be a string in all cases that don't result in an Exception. I think if you update the properties and helper function that they end up calling, that this check can go away.
There was a problem hiding this comment.
Fixed (I think). Please take a look at the new changes
There was a problem hiding this comment.
Looks good to me - thank you!
| @@ -672,6 +673,7 @@ def handlepullerror(e): | |||
| # We only pull if we are using symbolic names or the requested revision | |||
| # doesn't exist. | |||
| havewantedrev = False | |||
| checkoutrevision = None | |||
There was a problem hiding this comment.
Hooray, this fixes some cases where this is potentially undefined!
| @@ -51,45 +51,57 @@ def run(self, *args: str, **kwargs): | |||
| return "" | |||
| raise | |||
|
|
|||
| @abstractproperty | |||
There was a problem hiding this comment.
TIL that abstractproperty is deprecated. Thanks for updating this!
There was a problem hiding this comment.
No problem. It seems datetime.utcnow is as well, but I have left it unchanged.
|
Thank you so much for adding so many type hints @YaySushi! |
This fixes (a very small) part of issue #476.
Notes:
pytestreported the same stats before and after my changes.Some removals of
# type: ignoredid not result in an error from pyright (some status code comparisons)