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

Flatten column names and values with a comma-delimiter (#24) #73

Merged
merged 2 commits into from
Jul 2, 2022
Merged

Conversation

dm-p
Copy link
Contributor

@dm-p dm-p commented Jun 30, 2022

Applies to both Source and Destination node data roles.

e.g. if FirstName + LastName columns are supplied, this will name the field as `FirstName,LastName'

Values submitted to the request are then similarly flattened, e.g.:

["Daniel,Marsh-Patrick", "Desirree,Adegunle"]

This pattern will consolidate and delimit a smany columns as are supplied to each data role.

Applies to both Source and Destination node data roles.

e.g. if `FirstName` + `LastName` columns are supplied, this will name the field as `FirstName,LastName'

Values submitted to the request are then similarly flattened, e.g.:

`["Daniel,Marsh-Patrick", "Desirree,Adegunle"]`

This pattern will consolidate and delimit a smany columns as are supplied to each data role.
@dm-p dm-p requested a review from lmeyerov June 30, 2022 23:07
@@ -209,7 +212,8 @@ export class Visual implements IVisual {
);
return false;
}
for (let i = 0; i < this.prevValues[colName].length; i++) { // eslint-disable-line
for (let i = 0; i < this.prevValues[colName].length; i += 1) {
// eslint-disable-line
Copy link
Contributor

Choose a reason for hiding this comment

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

unintentional linebreak? or lint no longer needed?

Copy link
Contributor

@lmeyerov lmeyerov left a comment

Choose a reason for hiding this comment

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

lgtm

@lmeyerov
Copy link
Contributor

lmeyerov commented Jul 1, 2022

@dess890 can you try building and confirming generates graph topology + edge src/dest/properities as expected?

@lmeyerov lmeyerov requested a review from dess890 July 1, 2022 00:21
@dess890 dess890 self-requested a review July 2, 2022 00:11
Copy link
Contributor

@dess890 dess890 left a comment

Choose a reason for hiding this comment

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

tested -- everything is working!

@lmeyerov lmeyerov merged commit 419e8d1 into main Jul 2, 2022
@lmeyerov
Copy link
Contributor

lmeyerov commented Jul 2, 2022

Great, merging!

@dm-p feel free to continue or delete branch

@dm-p dm-p deleted the dm-p branch July 28, 2022 23:02
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.

3 participants