DjangoGirls / tutorial

This is a tutorial we are using for Django Girls workshops
http://tutorial.djangogirls.org/
Other
1.54k stars 1.86k forks source link

rename "post" (noun) to "blog post" #1648

Open das-g opened 4 years ago

das-g commented 4 years ago

As noted in https://github.com/DjangoGirls/tutorial/issues/1642#issue-562077173 and https://github.com/DjangoGirls/tutorial/issues/1642#issuecomment-583785230, there's three things that can be easily mistaken for oneanother:

Mixing up the latter two isn't really an issue, as they refer to the same concept anyway, only on different levels of abstraction. Mixing up the first with any of the others two can lead to profound confusion though, as the object referred to by the noun is the result of the actions referred to by the verbs.

To avoid this ambiguity, especially in code where one cannot rely on sentence structure to keep these words and concepts apart, let's base all variable names referring to the first (the noun) on "blog post" rather than just "post".

This means:

  1. rename class Post to BlogPost.
  2. rename variables and arguments holding an instance of Post from post to blog_post
  3. rename variables and arguments holding an QuerySet of Post from posts to blog_posts
  4. rename views, corresponding templates and URL "name"s (not the URLs path patterns themselves):
    • post_listblogpost_list
    • post_detailblogpost_detail
    • post_newnew_blogpost
    • post_editedit_blogpost
nikhiljohn10 commented 4 years ago

@das-g I have an alternative suggestion. Use namespace. It's very straight forward if you want to make multiple apps.

Simply add app_name = 'blogpost'

For example:

app_name = 'blogpost'
urlpatterns = [
    path('', views.list, name='list'),
    path('post/<int:pk>/', views.detail, name='detail'),
    path('post/new/', views.create, name='create'),
    path('post/<int:pk>/edit/', views.update, name='update'),
    path('psot/<int:pk>/delete/', views.delete, name='delete'),
]

with this, we can use the following:

{% url 'blogpost:list' %}
reverse('blogpost:list')

{% url 'blogpost:detail' %}
reverse('blogpost:detail')

{% url 'blogpost:create' %}
reverse('blogpost:create')

{% url 'blogpost:update' %}
reverse('blogpost:update')

{% url 'blogpost:delete' %}
reverse('blogpost:delete')

Of course, this needs an explanation about the namespace concept in a layman's term. What do you think about it?

I agree to rename the class with BlogPost instead of Post

das-g commented 4 years ago

I have an alternative suggestion. Use namespace.

I like that idea. :+1: It's quite in line with the Zen of Python (PEP 20), whose 19th aphorism proclaims:

Namespaces are one honking great idea -- let's do more of those!

nikhiljohn10 commented 4 years ago

@das-g I am pushing commits for namespace change as well in different PR: https://github.com/DjangoGirls/tutorial/compare/master...jwaladiamonds:namespace

nikhiljohn10 commented 4 years ago

Namespace related changes can not be merged unless the PR https://github.com/DjangoGirls/tutorial/pull/1681 is resolved. Otherwise, lots of conflicts are going to arise from it.

@ekohl Can you please look into that PR?

fltfx commented 1 month ago

Seems resolved in PR #1691

das-g commented 1 month ago

1691 was closed by its author without being merged. Re-opening thus.

fltfx commented 1 month ago

@das-g Okay, that makes sense to me. Like #1691 mentions, we probably should leave that PR closed and make a new one with the changes as there are many merge conflicts by now.