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.

Sign up for GitHub

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

Jump to bottom

tests_models.py:85-90: Try to implement... #537

Closed
0pdd opened this issue Aug 22, 2018 · 4 comments
Closed

tests_models.py:85-90: Try to implement... #537

0pdd opened this issue Aug 22, 2018 · 4 comments
Labels
3 we can defer this issue. But it'll good to implement it cleanup everything that do project stronger, flexible, reusable wontfix

Comments

@0pdd
Copy link
Collaborator

0pdd commented Aug 22, 2018

The puzzle 525-5d17d280 from #525 has to be resolved:

shopelectro/shopelectro/tests/tests_models.py

Lines 85 to 90 in 64939d2

@todo #525:30m Try to implement Product.objects.get_brands()
Currently we use Tag.objects.get_brands(), be it seems is not so convenient
as it may be.
Details:
https://github.com/fidals/shopelectro/pull/531#discussion_r211858857
https://github.com/fidals/shopelectro/pull/531#discussion_r211962280

The puzzle was created by Artemiy on 22-Aug-18.

Estimate: 30 minutes,

If you have any technical questions, don't ask me, submit new tickets instead. The task will be "done" when the problem is fixed and the text of the puzzle is removed from the source code. Here is more about PDD and about me.
@0pdd
Copy link
Collaborator Author

0pdd commented Aug 22, 2018

@artemiyds2 @duker33 please pay attention to this new issue.

0pdd referenced this issue Aug 22, 2018
@ArtemijRodionov
#525 Create Tag.get_brands and related tests (#531)
64939d2 
Implement js-part for brand extraction

Self-review fixes

Fix test

Apply linter rules

The first review fixes

Create todo for Products.get_brands()
This was referenced Aug 22, 2018
Closed
Closed
@ArtemijRodionov
Copy link
Contributor

I'd prefer to leave Tag.objects.get_brands(), because a brand is Tag object

@ArtemijRodionov ArtemijRodionov added 30 mins middle-sized issue. Usually a part of feature or a fix cleanup everything that do project stronger, flexible, reusable discuss issue needs to finish discussion before start working 3 we can defer this issue. But it'll good to implement it labels Aug 22, 2018
@ArtemijRodionov ArtemijRodionov mentioned this issue Sep 6, 2018
Merged
@duker33
Copy link
Contributor

duker33 commented Sep 7, 2018 •
edited

@artemiy312 , maybe then Product.objects.get_brands() ?
Pros:

  • we don't needed method arg products in client code
  • we don't needed in Tag class in client code: Tag.objects.get_brands(products.get_brands()) -> products.get_brands()

@ArtemijRodionov
Copy link
Contributor

@duker33

products.get_brands() maybe looks more convenient, but will work ineffective as i showed through test

@duker33 duker33 removed the discuss issue needs to finish discussion before start working label Sep 8, 2018
@duker33 duker33 self-assigned this Sep 8, 2018
@duker33 duker33 removed their assignment Nov 4, 2019
@duker33 duker33 added wontfix and removed 30 mins middle-sized issue. Usually a part of feature or a fix labels Nov 4, 2019
@duker33 duker33 closed this as completed Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3 we can defer this issue. But it'll good to implement it cleanup everything that do project stronger, flexible, reusable wontfix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@duker33 @ArtemijRodionov @0pdd