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

Visitor Identification module added. #28

Merged
merged 6 commits into from
Jan 10, 2024

Conversation

Sehbaz
Copy link
Contributor

@Sehbaz Sehbaz commented Dec 19, 2023

What to do

Added visitor identification API code to the HubspotChat component.
Referenced HubSpot documentation on visitor identification.

Background

The modification was necessary to include the visitor identification API code, which was missing from the library.
The code has been added based on the information available in the HubSpot documentation.

Acceptance criteria

The added code for visitor identification functions as intended.
The linting and testing processes pass successfully during GitHub Actions.

@kk-no kk-no self-requested a review December 21, 2023 00:45
@Sehbaz
Copy link
Contributor Author

Sehbaz commented Dec 21, 2023

@kk-no I have updated the code for the last test fail case, kindly check and approve for test again.

@kk-no
Copy link
Member

kk-no commented Dec 22, 2023

@Sehbaz
Thank you for the pull request. I re-run the test, but it still seems to fail...
Since it's the end of the year, the review may be a little late, so please bear with me.

Copy link
Member

@kk-no kk-no left a comment

Choose a reason for hiding this comment

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

I made a comment, so please check it as well as fix the test failure.

gohubspot.go Outdated Show resolved Hide resolved
visitor_identification.go Outdated Show resolved Hide resolved
@Sehbaz Sehbaz marked this pull request as draft January 3, 2024 01:40
@Sehbaz Sehbaz marked this pull request as ready for review January 3, 2024 17:47
@Sehbaz
Copy link
Contributor Author

Sehbaz commented Jan 3, 2024

@kk-no I have update my code and now that looks far more clean and that should take care of test cases too.

I have made PR ready to be review again.

Thank you for valuable feedback.

@Sehbaz Sehbaz requested a review from kk-no January 3, 2024 17:48
Copy link
Member

@kk-no kk-no left a comment

Choose a reason for hiding this comment

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

Thank you for your update.
It's almost perfect, but I'd like to make the format a little more consistent, so I made a comment. If that is fixed, I think I can merge it!

conversation.go Outdated Show resolved Hide resolved
conversation_visitor_identity.go Show resolved Hide resolved
conversation_visitor_identity.go Outdated Show resolved Hide resolved
@Sehbaz Sehbaz marked this pull request as draft January 4, 2024 15:13
@Sehbaz Sehbaz marked this pull request as ready for review January 7, 2024 17:58
@Sehbaz
Copy link
Contributor Author

Sehbaz commented Jan 7, 2024

@kk-no I have update my code and I have made PR ready to be review again.

Thank you for valuable feedback.

@Sehbaz Sehbaz requested a review from kk-no January 7, 2024 17:59
Copy link
Member

@kk-no kk-no left a comment

Choose a reason for hiding this comment

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

LGTM!
We will release v0.8.0 including this OR in the soon.

@kk-no kk-no merged commit 993402a into belong-inc:main Jan 10, 2024
1 check passed
@Sehbaz
Copy link
Contributor Author

Sehbaz commented Jan 11, 2024

LGTM! We will release v0.8.0 including this OR in the soon.

Amazing, thanks for an update!

@kk-no kk-no mentioned this pull request Feb 8, 2024
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