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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

tbh your wrap per sucks #1

Closed
fourjr opened this issue Feb 18, 2018 · 8 comments
Closed

tbh your wrap per sucks #1

fourjr opened this issue Feb 18, 2018 · 8 comments
Assignees
Labels
TODO Something that is going to be worked on

Comments

@fourjr
Copy link
Contributor

fourjr commented Feb 18, 2018

  1. loop and session should be grabbed as a kwarg in Client init 馃憤
  2. __str__ should be a method for player and band, etc 馃憤
  3. self.timeout = options.get('timeout') should have a default of 5 馃憤
  4. Tags should be pre-checked instead of polling the API and getting an invalid tag 馃憤
  5. from .errors import * should not be a wildcard import 馃憤
  6. more of a personal preference, all errors should include endpoint that includes tag 馃憤
band = Band(raw_data, camel_killer_box=True)
return band

what's the use of creating a var here 馃憤
8. why specify camel_killer_box=True) all the time? why not just create a box base class for your wrapper where you specify that in the init? 馃憤
9. get_player should be alias for get_profile 馃憤
10. timeout: Optional[int] Quits requests to the API should be **timeout in docstrings 馃憤
11. get the request code for unauthorized 401? 馃
think hope that's it
12. in __init__.py:

from .core import *
from .errors import *
from .utils import API

import utils isn't needed here? 馃憤
13. send only one message in the dpy cog example (readme.md) 馃憤
14. in all docstrings, for kwargs use **varname instead of varname 馃憤
15. Get your token by DMing Zihad#6591 on Discord or ask in the聽API Server. update plox 馃憤

@fourjr
Copy link
Contributor Author

fourjr commented Feb 18, 2018

forgot this:
except abrawlpy.errors.RequestError as e: according to your init.py, except abrawlpy.RequestError should be good enough 馃憤

@SharpBit
Copy link
Owner

SharpBit commented Feb 18, 2018

  1. Wildcard error import? Wdym by this, cuz all errors were used in the file
  2. for __str__ I just inherit it from box class
  3. "All errors should include endpoint that includes tag": ???
  4. By prechecked do you mean: upper case, replace O with 0, and make sure only tag characters are used?

@fourjr
Copy link
Contributor Author

fourjr commented Feb 18, 2018

  1. it's never good to have a wildcard import, as stated in pep8
  2. have a str for player like: 4JR (#2P0LYQ)
  3. Errors are vague, and not good for production, it should at least include the endpoint which was requested
  4. yes.

@SharpBit
Copy link
Owner

Why does the endpoint matter? Doesn't the error state where it was (such as get_band)

@fourjr
Copy link
Contributor Author

fourjr commented Feb 18, 2018

it's easier to debug when you have the tag used that caused the error

@SharpBit
Copy link
Owner

SharpBit commented Feb 18, 2018

Does pep8 really matter when it's less characters and words? :/ Saying * is way easier than listing every single error

@fourjr
Copy link
Contributor Author

fourjr commented Feb 18, 2018

pep8 always matters ;)
but it's much clearer as to what's imported and where stuff are defined

@SharpBit SharpBit changed the title tbh your wrap per sucks tbh your wrap per sucks (tbh your gram mar sucks) Feb 18, 2018
@SharpBit SharpBit self-assigned this Feb 18, 2018
@SharpBit SharpBit added the TODO Something that is going to be worked on label Feb 18, 2018
@fourjr fourjr mentioned this issue Feb 18, 2018
13 tasks
@SharpBit
Copy link
Owner

fixed everything in here and your commit comments, if you find more issues, open another issue

@SharpBit SharpBit changed the title tbh your wrap per sucks (tbh your gram mar sucks) tbh your wrap per sucks Dec 16, 2018
@SharpBit SharpBit assigned SharpBit and unassigned SharpBit Dec 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO Something that is going to be worked on
Projects
None yet
Development

No branches or pull requests

2 participants