Skip to content

Added types to Index submodule #1244

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 14 commits into from
May 18, 2021
Merged

Added types to Index submodule #1244

merged 14 commits into from
May 18, 2021

Conversation

Yobmod
Copy link
Contributor

@Yobmod Yobmod commented May 15, 2021 •
edited
Loading

Added types to Index submodule,
plus change some types in othe files to make them agree

@Yobmod Yobmod marked this pull request as draft May 15, 2021 22:15
@Byron
Copy link
Member

Byron commented May 16, 2021 •
edited
Loading

@Yobmod Thanks a lot! Do you know how it's possible that somebody I didn't assign can approve changes and do so repeatedly? Is that a GitHub bug? Should I be concerned?

Update: Just to be on the safe side, I blocked the user for 30days. Once we know there was no ill-intend and there is no security hole, I am happy to undo this action of course. After checking organization and repository settings I couldn't see how reviewing seemingly random PRs is possible.

@Yobmod
Copy link
Contributor Author

Yobmod commented May 16, 2021 •
edited
Loading

Oh, don't know how that is possible (or who the user is).
It's never happened until I used the draft feature. Maybe something to do with that?

If it happens again, I can close this one and get it ready for review offline.

Byron
Byron approved these changes May 17, 2021
View reviewed changes
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for yet another fantastic addition of types.

All changes to the logic where based on the type-system recognising that some fields or variables might be None, so I wouldn't expect any issues arising from this.

git/index/base.py Outdated
"""
:return:
Iterator yielding dict(path : list( tuple( stage, Blob, ...))), being
a dictionary associating a path in the index with a list containing
sorted stage/blob pairs
##### Does it return iterator? or just the Dict?
Copy link
Member

Choose a reason for hiding this comment

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

It really does look like it returns a dict, and the type system probably agrees.
In these cases it should be fine to fix the documentation, which rightfully is confused about types in a codebase that thus far didn't have any statically declared.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, i changed the docstring then

@Byron
Copy link
Member

Byron commented May 17, 2021

Please let me know if this is ready for merging, the PR appears to be in draft mode still.

@Yobmod Yobmod changed the title Add initial types to IndexFile .init() to _to_relative_path() Added types to Index submodule May 17, 2021
@Yobmod
Copy link
Contributor Author

Yobmod commented May 17, 2021 •
edited
Loading

Now it's ready!

@Yobmod Yobmod marked this pull request as ready for review May 17, 2021 12:16
@Byron Byron added this to the v3.1.18 - Bugfixes milestone May 18, 2021
@Byron
Copy link
Member

Byron commented May 18, 2021

This looks good to me, thanks a lot!

@Byron Byron merged commit b11bcfa into gitpython-developers:main May 18, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Byron Byron Byron approved these changes

@d066hie d066hie d066hie approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
v3.1.18 - Bugfixes
Development

Successfully merging this pull request may close these issues.
3 participants
@Yobmod @Byron @d066hie