studybuffalo / django-flexible-subscriptions

A subscription and recurrent billing application for Django.
GNU General Public License v3.0
249 stars 57 forks source link

Added configurations for brazilian real (BRL) currency #160

Closed ronflima closed 4 years ago

ronflima commented 4 years ago

Added formatting for BRL currency according to code documentation.

codecov[bot] commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@e7fd277). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             master      #160   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?         8           
  Lines             ?       721           
  Branches          ?        35           
==========================================
  Hits              ?       721           
  Misses            ?         0           
  Partials          ?         0           
Impacted Files Coverage Δ
subscriptions/abstract.py 100.00% <0.00%> (ø)
subscriptions/admin.py 100.00% <0.00%> (ø)
subscriptions/apps.py 100.00% <0.00%> (ø)
subscriptions/models.py 100.00% <0.00%> (ø)
subscriptions/forms.py 100.00% <0.00%> (ø)
subscriptions/management/commands/_manager.py 100.00% <0.00%> (ø)
subscriptions/views.py 100.00% <0.00%> (ø)
subscriptions/templatetags/currency_filters.py 100.00% <0.00%> (ø)

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 e7fd277...8d5932b. Read the comment docs.

ronflima commented 4 years ago

Thanks for the contribution! I noticed a few extra new lines were added to the file that I have flagged for removal just to keep the styling consistent. If you have a moment to take those out it would be appreciated. Afterwards this is good for approval and I can get a new version out.

I´ll do that. I´m using autopep8 to force code styling consistent with python standards. I´ll check if this was forced by it.

ronflima commented 4 years ago

Just made requested changes. I have added, also, .pep8 configuration to avoid autopep8 break stablished code formatting by adding 2 lines at the end of each function or class (E305). Also, added a file to .gitignore to avoid saving an emacs configuration file to the repo.

studybuffalo commented 4 years ago

Ah thanks for the extra info. I must have missed/forgot about 2 newlines between functions per PEP8. I try to keep this package in line with PEP wherever possible and will make sure to fix that.

To keep the git history clear/consistent, would you be able to pull out the .pep8 file and I will do a new PR in the next week or two to bring the package into alignment with this PEP8 standard? Sorry to be a pain and I really appreciate you bringing it to my attention!

ronflima commented 4 years ago

Don´t worry. I have added the .pep8 only to force autopep8 to follow your standards. Auto-formatters are great to ensure code style compliance. I´ll take it out from the commit. BTW, if you want, I can format the entire code to pep8 standards. It is a matter of running a script to automate this task. But I will add it in another PR. Let me know if you want it.

studybuffalo commented 4 years ago

Thanks! It is 100% your call if you'd like to update the formatting to align. I guess the one caveat I will add: there are probably some deviations made that align more with Django styles than Python. I tried to stick to Python standards wherever possible, but there may have been some areas where I felt Django was more readable. I may be a bit nitpicky around those areas, but hopefully it doesn't come up too often if I stuck to the style guide as intended!

studybuffalo commented 4 years ago

Also, I will try and get a new version out this weekend to have these changes added to the package. Today is "dependency update" day, so just want to get those all done before putting out a new version in case there are any dependency conflicts I need to resolve (unlikely, but you never know).