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

added constants package for all constants #2076

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

shaysw
Copy link
Collaborator

@shaysw shaysw commented Jan 15, 2022

Took all the (inner) classes in BE consts + constants.py and split/put them in dedicated .py files, one for each class (except for inner classes in BE consts)

Took all the (inner) classes in BE consts + constants.py and split/put them in dedicated .py files, one for each class (except for inner classes in BE consts)
@codecov
Copy link

codecov bot commented Jan 15, 2022

Codecov Report

Merging #2076 (3a3418e) into dev (b7eba15) will increase coverage by 0.51%.
The diff coverage is 75.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2076      +/-   ##
==========================================
+ Coverage   51.85%   52.36%   +0.51%     
==========================================
  Files         100      110      +10     
  Lines        7942     8059     +117     
==========================================
+ Hits         4118     4220     +102     
- Misses       3824     3839      +15     
Flag Coverage Δ
unittests 52.36% <75.17%> (+0.51%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
anyway/accidents_around_schools.py 0.00% <0.00%> (ø)
anyway/constants/constants.py 96.42% <ø> (ø)
anyway/parsers/cbs/executor.py 0.00% <0.00%> (ø)
anyway/parsers/cbs/s3/data_retriever.py 0.00% <0.00%> (ø)
anyway/parsers/injured_around_schools.py 0.00% <0.00%> (ø)
anyway/parsers/rsa.py 0.00% <0.00%> (ø)
anyway/widgets/suburban_widgets/__init__.py 100.00% <ø> (ø)
..._widgets/accident_count_by_accident_type_widget.py 89.28% <ø> (ø)
...rban_widgets/accident_count_by_day_night_widget.py 89.47% <0.00%> (ø)
.../suburban_widgets/accident_count_by_hour_widget.py 89.47% <ø> (ø)
... and 52 more

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 59ac1f6...3a3418e. Read the comment docs.

@atalyaalon
Copy link
Collaborator

@shaysw looks good, there is a small conflict in user_system/api.py file - can you resolve?

@BarVolunteering
Copy link
Collaborator

@shaysw What do you think about that:
image

from flask_babel import _


class BackEndConstants(object):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are doing a big change, I do not see value in having the constants in this file inside a class.

Copy link
Collaborator

@ziv17 ziv17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

@shaysw shaysw marked this pull request as draft January 20, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants