Skip to content
This repository was archived by the owner on Dec 7, 2022. It is now read-only.

ABC Form Messaging Dev Doc#2002

Open
ppadture wants to merge 30 commits intoLivePersonInc:masterfrom
ppadture:master
Open

ABC Form Messaging Dev Doc#2002
ppadture wants to merge 30 commits intoLivePersonInc:masterfrom
ppadture:master

Conversation

@ppadture
Copy link
Contributor

@ppadture ppadture commented Oct 26, 2022

ABC Form Messaging Dev Doc

@ppadture ppadture marked this pull request as draft October 27, 2022 09:31
@ppadture ppadture marked this pull request as ready for review October 27, 2022 09:32
@j9t j9t self-requested a review October 28, 2022 09:30
j9t
j9t suggested changes Oct 28, 2022
View reviewed changes
Copy link
Contributor

@j9t j9t left a comment

Choose a reason for hiding this comment

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

Please also rename the images to use a lowercase extension (“png”).

See our content and code guide for general guidance. Thank you!

@ppadture ppadture requested a review from j9t October 28, 2022 11:18
@ppadture ppadture requested review from cbikar and j9t and removed request for cbikar and j9t October 28, 2022 12:24
j9t
j9t suggested changes Oct 28, 2022
View reviewed changes
Copy link
Contributor

@j9t j9t left a comment

Choose a reason for hiding this comment

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

Please rename the images as mentioned (“png”).

j9t
j9t suggested changes Oct 28, 2022
View reviewed changes
Copy link
Contributor

@j9t j9t left a comment

Choose a reason for hiding this comment

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

Nice work! Almost there.

When do you prefer to have this live, immediately after the review is done?

j9t
j9t suggested changes Oct 31, 2022
View reviewed changes
Copy link
Contributor

@j9t j9t left a comment

Choose a reason for hiding this comment

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

Checked on the PR on an indentation question—is that still open?

@ppadture ppadture requested a review from j9t November 2, 2022 08:13
@ppadture
Copy link
Contributor Author

ppadture commented Nov 2, 2022

@j9t i am not sure what do you mean by unindent

@ppadture ppadture requested a review from j9t November 2, 2022 08:20
j9t
j9t approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@j9t j9t left a comment

Choose a reason for hiding this comment

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

LGTM—thanks for your work on this.

“Unindent”—it seems you fixed it: It related to a paragraph that was indented, but shouldn’t be. Hence I asked to “un-indent” it.

@j9t
Copy link
Contributor

j9t commented Nov 2, 2022

(Feel free to merge whenever you like, or if you need us to merge, to let me know when this can be merged.)

@ppadture
Copy link
Contributor Author

ppadture commented Nov 4, 2022

@j9t the changes from this branch should not go live immediately, the development for LE-UI is still in progress

@j9t
Copy link
Contributor

j9t commented Nov 4, 2022

Thanks for the update! We’re there if you need anything.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@cbikar cbikar cbikar left review comments

@j9t j9t j9t approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ppadture @j9t @cbikar @itay1313