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

Missing stubs #467

Merged
merged 5 commits into from
Oct 11, 2020
Merged

Missing stubs #467

merged 5 commits into from
Oct 11, 2020

Conversation

Goldziher
Copy link
Member

Add missing stubs

This PR builds on my previous PR (it includes it diff, but if the other PR is merged the diff here will be reduced in size). I added the missing stub files (minus migrations) from Django 3.1.1, there is still some missing data inside the existing stubs though.

@kszmigiel
Copy link
Member

Wow, that's a lot of work, thank you!
I'm not sure if we can make Django 2.2 tests pass before we find out a proper way of stubs versioning. @sobolevn what do you think?

@sobolevn
Copy link
Member

I'm not sure if we can make Django 2.2 tests pass before we find out a proper way of stubs versioning. @sobolevn what do you think?

Yes, sure.

@Goldziher
Copy link
Member Author

Hi, @kszmigiel , I will see if I can resolve those issues on Saturday.

@Goldziher
Copy link
Member Author

I removed form the PR the contrib.postgres.forms - because this is what was failing and there is already an approved pending PR that adds it (https://github.com/typeddjango/django-stubs/pull/289/files).

@Goldziher
Copy link
Member Author

Hi guys. 19 days is a long time to wait for a review or an approval...

Copy link
Member

@sobolevn sobolevn 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!

I can tell you honestly: I cannot review 180+ stub files and find any potential mistakes in the API that I don't use. But!

My general question is about the number of Any you use in this code.
Can we somehow reduce it? Take this an example:

class FlatPageAdmin(admin.ModelAdmin):
    form: Any = ...
    fieldsets: Any = ...
    list_display: Any = ...
    list_filter: Any = ...
    search_fields: Any = ...

It does not have even a single specific type 😞
I guess it is still better than no stub at all, but maybe we can do something easy to improve the quality of typings here?

@Goldziher
Copy link
Member Author

@sobolevn my intention is to first amend the no stubs at all and then gradually help in fixing these. I too cannot type so many files 😃. I use DRF professionally for the most part.

The data was generated by stubgen and I fixed it against the tests.

@sobolevn
Copy link
Member

sobolevn commented Oct 9, 2020

@kszmigiel can you please take a look? Maybe you can spot something? If not, then let's merge it!

@Goldziher thanks again for your massive contribution! It is awesome.

@Goldziher
Copy link
Member Author

Can we merge this? I'd like to make a new PR.

@sobolevn sobolevn merged commit ce370ea into typeddjango:master Oct 11, 2020
@sobolevn
Copy link
Member

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants