sports-alliance / sports-lib

A Library for processing GPX, TCX, FIT and JSON files from services such as Strava, Movescount, Garmin, Polar etc
GNU Affero General Public License v3.0
149 stars 21 forks source link

Group activity types #42

Closed jimmykane closed 4 years ago

jimmykane commented 4 years ago

The ultimate purpose of this PR is to group activities in order for consuming projects to be able to apply eg coloring by "outdoor" activities.

I am opening from now the PR just to debate on a few things, since it does contain some big refactoring.

I added some @todo points that explain / ask a few this time ;-)

I would though to explain the Activity types enum.

The ActivityTypes enum has many different keys that point to the same value. That is in order to access fast and catch as many "typos" different input and produce one uniformal value. I added that to a small doc block above each "match" group. This also allows flexibility on accessing values code wise. Eg ActivityTypes.TrailRunning is more nice to have in code than ActivityTypes['Trail Running'] so a key of TrailRunning could be added to the enum.

Now this is what I think but if you have a better approach i'd be happy to refactor more.

codecov-io commented 4 years ago

Codecov Report

Merging #42 into develop will increase coverage by 0.55%. The diff coverage is 99.44%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #42      +/-   ##
===========================================
+ Coverage    71.62%   72.17%   +0.55%     
===========================================
  Files          172      172              
  Lines         4243     4331      +88     
  Branches       641      645       +4     
===========================================
+ Hits          3039     3126      +87     
- Misses        1189     1190       +1     
  Partials        15       15
Impacted Files Coverage Δ
src/activities/activity.types.ts 94.77% <99.44%> (+1.14%) :arrow_up:

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 6944ac8...bfd9250. Read the comment docs.

jimmykane commented 4 years ago

I am not sure if the types should be upper cased.... @thomaschampagne what do you prefer?

Trail running Trail Running

jimmykane commented 4 years ago

@thomaschampagne I think this is ready.

I left some @todo for the groups + activity types so feel free to add more activity types to groups.

There is also a question.

How should be the activity name values ? camelCase? UpperCase? Firstupper?

thomaschampagne commented 4 years ago

@jimmykane I'm starting the review. I would say UpperCase but it's just my point of view. I let you the final choice ;)

jimmykane commented 4 years ago

Fixed those little issues.

Now to figure out how to force webstorm to use spaces for imports.

jimmykane commented 4 years ago

Issue with the editor is fixed.

thomaschampagne commented 4 years ago

Do you need some help to finish the "UpperCasing" before merging?

jimmykane commented 4 years ago

On it now :-D

jimmykane commented 4 years ago

@thomaschampagne should be good to go.