-
Notifications
You must be signed in to change notification settings - Fork 10.4k
[IMP] email marketing: update essentials doc #7521
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
Conversation
efcc8b8 to
a25d2c8
Compare
|
@meng-odoo This one is ready for its first round of Peer Review, whenever you get a chance! Thanks! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo, great job on this doc, it's very detailed! I had a bunch of tiny wording suggestions/typo fixes for you, then it will be good to go :)
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
content/applications/marketing/email_marketing/email_marketing_essentials.rst
Outdated
Show resolved
Hide resolved
a25d2c8 to
1471583
Compare
|
Thanks for the super-detailed feedback, @meng-odoo -- I really appreciate it. 🙏 I made all those suggested edits, and believe this one is now ready for Tech Review from you, @samueljlieber ! Thanks! 👍 |
1471583 to
a2af7d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo, awesome job updating the content of email_marketing.rst. There is a lot of good information here, and I think you covered the content very well. On the technical end of things, the doc looks pretty great! I only have a small handful of corrections, please see below.
Tag me for another look once these are addressed, thank you!
a2af7d8 to
758a2f7
Compare
|
Thanks for the always-helpful feedback, @samueljlieber 🙏 I made all the suggested edits, and believe this one is ready for another quick look from you, whenever you get a chance. Thanks! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo, thank you for implementing my changes. I caught just a few more technical corrections, please see below. Please tag me for one more quick look once these are addressed, thank you!
758a2f7 to
a090277
Compare
|
Alrighty @samueljlieber -- i made all those suggested edits. if you could add some clarification re: using a GUI for an app name in the corner of the UI, as opposed to the previously-used italics, that would be super helpful. Thanks! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @ksc-odoo, this PR looks good to me! Nice job on this one!
@StraubCreative this PR is good to go 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo
Very detailed form coverage, I love it!
Going to pause momentarily as I'm catching some high-level details that could use your attention around framing and emoji use. Can you take a look at these please and then tag @samueljlieber again for another review? Thanks!
a090277 to
02ed5b5
Compare
|
Thanks for all the super-helpful feedback, @StraubCreative -- made all those suggested edits, and now, believe it's ready for another look. So, @samueljlieber -- this one's ready for ya whenever you get a chance. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ksc-odoo! I gave this PR another look and found a few more corrections and suggestions. I am approving now, but please address these before moving the PR along. Other than these points this doc looks great, nice job 🙂
02ed5b5 to
0767ff9
Compare
|
Thanks for the helpful feedback, @samueljlieber -- made all the necessary adjustments, so I think this one is ready for you to officially merge it! |
0767ff9 to
474a8bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@ksc-odoo @samueljlieber this pull request has forward-port PRs awaiting action (not merged or closed): |
PROJECT TASK: https://www.odoo.com/web#id=3699244&cids=3&menu_id=4720&action=333&active_id=3835&model=project.task&view_type=form
Complete update of the Email Marketing doc.
Updated toctree slightly, so the Email Marketing Essentials (formerly Email Marketing) appears beneath the category selection.
Added a link to tutorial on the 'Email Marketing' category page