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

Trial/activity type #113

Merged
merged 7 commits into from
Aug 7, 2019
Merged

Trial/activity type #113

merged 7 commits into from
Aug 7, 2019

Conversation

merlos
Copy link
Owner

@merlos merlos commented Aug 6, 2019

Hi @vincentneo

Although it is a bit advanced for a regular user I am fine with exposing the activity type to the user.

I changed some of the activity types and added a short description for each mode to try to ease the selection.

BTW, I really like the way you used CLActivityType+Info.swift to display the options.

@merlos merlos requested a review from vincentneo August 6, 2019 01:33
@merlos
Copy link
Owner Author

merlos commented Aug 6, 2019

#66

Copy link
Collaborator

@vincentneo vincentneo left a comment

Choose a reason for hiding this comment

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

I think that it would be better practice to add in documentation for the codes before merge. Will try to add them bit by bit as time permits.

I didn't do so at first, as I didn't mean to make the branch as a pull request. (Though testing it gave quite inconclusive results)

The descriptions look real good though, as they provide more information for the average user.

Also, CLActivityType+Info was really inspired/mirrored from GPXTileServer's style.

@merlos
Copy link
Owner Author

merlos commented Aug 6, 2019

Good point about the documentation, I forgot about it.

I will be able to do some tests of this feature next week as I´ll be traveling.

@merlos
Copy link
Owner Author

merlos commented Aug 7, 2019

@vincentneo doc is now included 👍

@vincentneo
Copy link
Collaborator

Looking good now! Will merge.

@vincentneo vincentneo merged commit 6aa30ba into master Aug 7, 2019
@vincentneo vincentneo mentioned this pull request Aug 7, 2019
@merlos merlos deleted the trial/ActivityType branch October 11, 2019 23:36
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.

None yet

2 participants