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

Naming things #44

Open
1 of 7 tasks
crccheck opened this issue Dec 5, 2015 · 1 comment
Open
1 of 7 tasks

Naming things #44

crccheck opened this issue Dec 5, 2015 · 1 comment

Comments

@crccheck
Copy link
Owner

crccheck commented Dec 5, 2015

I could have sworn I had a naming things issue before. I'm not that happy with some of the name choices I made.

  • INSTALLED_APPS name: django_object_actions stays the same
  • main admin mixin: DjangoObjectActions
  • admin mixin w/o template: BaseDjangoObjectActions
  • model admin attribute: objectactions ➡️ change_actions ?
  • decorator: takes_instance_or_queryset ➡️ to_queryset ?
  • template context variable: objectactions ➡️ ?
  • docs: "tools" ➡️ "change actions", "action tools"

This is really the only thing blocking a 1.0 release

model admin attribute, objectactions

The scope of the project is expanding a bit. The Django admin has conventions for standard admin views. The docs show there's the:

  • add view
  • change view
  • changelist view
  • delete view
  • history view

Each of those could potentially have their own sets of actions. If we re-use the existing names, we end up with add_actions, change_actions, changelist_actions, delete_actions, and history_actions.

template context variable: objectactions

For every view, it'd be nice to have the same context variable. We don't need to differentiate between each kind.

@crccheck crccheck added this to the 1.0 milestone Dec 5, 2015
@crccheck crccheck mentioned this issue Oct 17, 2017
@gatsinski
Copy link

I just found this project. It works as expected and I'm going to use it in a lot of places. Thanks a lot. I'm also willing to help but I have some difficulties understanding the checklist.

If the name in INSTALLED_APPS remains the same, the checkbox should be checked, right?

Renaming the takes_instance_or_queryset decorator to just to_queryset seems like an easy task and I can do it if this is all that needs to be done.

About the objectactions context variable I have some suggestions:
custom_actions, actions_list, admin_actions and custom_admin_actions.

Actually, I don't think objectactions is that bad, except for the missing underscore. The first time I read it like objections. Maybe object_actions will be better.

DjangoObjectActions can be renamed to CustomActionsMixin or CustomObjectActionsMixin. The same goes with BaseDjangoObjectActions.

About the docs, change actions sounds nice.

@crccheck crccheck added the to do label Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants