Skip to content

Name developer field with data index#5

Open
dpwiese wants to merge 2 commits intoupdate-master-from-parse-fitfrom
name-developer-field-with-data-index
Open

Name developer field with data index#5
dpwiese wants to merge 2 commits intoupdate-master-from-parse-fitfrom
name-developer-field-with-data-index

Conversation

@dpwiese
Copy link
Collaborator

@dpwiese dpwiese commented Nov 13, 2019

This (WIP) PR appends the name of developer data fields with their corresponding developer_data_index. The reason is that more than one developer data field may be used, each that has fields with the same name. In the current implementation, only one of these fields will be populated in each record.

This PR (inelegantly) differentiates between these multiple developer data fields by appending the developer_data_index to the end of the name.

@dpwiese dpwiese self-assigned this Nov 13, 2019
@dpwiese dpwiese requested a review from lukerayman November 13, 2019 16:33
@jimmykane
Copy link

Hey hi!

This is a great change but could you show some examples? I am curious

@dpwiese
Copy link
Collaborator Author

dpwiese commented Jan 3, 2020

@jimmykane - I'm not sure how much I can help beyond what is in the PR description above, but these changes were a quick and dirty workaround to using easy-fit to parse FIT files which may contain multiple developer data fields which all have the same field_name.

With the above changes each field_name, let's call it my-field-name, would be appended with its corresponding developer_data_index resulting in three uniquely named fields of my-field-name-0, my-field-name-1, and my-field-name-2. Thus each of these now uniquely named developer data fields could be had along with their corresponding data and easily differentiated.

These changes have served their purpose for now, although it's not an elegant solution to this problem and I've yet to have time to revisit it since. Hope this helps!

@jimmykane
Copy link

@dpwiese got I think I understand what you mean. I ll give it a try / check and perhaps merge it to fit-file-parser (updated npm for easyfit) and also to the typescript project we work with Piere

@dpwiese
Copy link
Collaborator Author

dpwiese commented Jan 4, 2020

tbh, I'd not feel great about merging as-is until this particular problem can be given more attention. I'd expect there to be a more elegant way to solve this problem. Perhaps return an array of identically named developer data fields?

@jimmykane
Copy link

Ok thanks!

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.

2 participants