AndrewIngram / django-extra-views

Django's class-based generic views are awesome, let's have more of them.
MIT License
1.39k stars 172 forks source link

Big Documentation Clean Up #178

Closed sdolemelipone closed 5 years ago

sdolemelipone commented 6 years ago

This is a clean up and expansion of the documentation. It's the first time I've done any restructuring of docs so please let me know if you see any improvements needed. Maybe the formset views section is so large that they should be divided up into more manageable pages.

I didn't modify the ListView documentation much as I'm not sure what their future is.

sdolemelipone commented 6 years ago

One of the deliberate changes I made was to reduce the prominence of GenericInline formsets in documentation as this sometimes seemed to confuse new people who think "yeah, I'd like a generic view".

codecov-io commented 6 years ago

Codecov Report

Merging #178 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #178   +/-   ##
=======================================
  Coverage   50.88%   50.88%           
=======================================
  Files           6        6           
  Lines         509      509           
  Branches       56       56           
=======================================
  Hits          259      259           
  Misses        234      234           
  Partials       16       16

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 52d9f7a...db10675. Read the comment docs.

sdolemelipone commented 6 years ago

I'd also add that I think all issues flagged as documentation can be closed after this with the exception of #52, which still needs completing.

jonashaag commented 6 years ago

I'll need a few days to review this. Does RTD support PRs? Otherwise I'll have to build it locally.

sdolemelipone commented 6 years ago

It only supports branches/tags on the main repo as far as I can see. The other option would be to create a new branch, switch the target of my pull request to that branch and accept the PR there, then review on RTD.

That might be a good idea just because I haven't seen how it builds on RTD, only locally.

jonashaag commented 6 years ago

Yeah, why not!

sdolemelipone commented 6 years ago

Great! Let me know if you need me to modify the PR once you've created the new branch.

jonashaag commented 6 years ago

I wanted to give you commit permission to the branch but I don't have admin permissions, @AndrewIngram can you give me admin?

jonashaag commented 6 years ago

I pushed it as pr-178 branch

AndrewIngram commented 6 years ago

I don't think I can control collaborator permissions without moving the repo over to an organisation, but i've sent an invite to @sdolemelipone

jonashaag commented 6 years ago

Thanks

sdolemelipone commented 6 years ago

@auvipy thanks for the review!

sdolemelipone commented 6 years ago

@jonashaag I'm just waiting for the new pr-178 branch to appear on read the docs then we can take a look at this there as well...I imagine there is a delay in the site picking up a new active branch.

sdolemelipone commented 6 years ago

RTD version available here: https://django-extra-views.readthedocs.io/en/pr-178/

jonashaag commented 6 years ago

Great stuff!! Feel free to merge anytime.

Maybe we could add a very short list of features in the intro:

sdolemelipone commented 5 years ago

Thanks, I'll add that in and merge. After that may I go through and close the documentation issues which this addresses?

jonashaag commented 5 years ago

Yep :)