openedx / edx-documentation

http://docs.edx.org
Other
163 stars 338 forks source link

doc: Explains the conditionals for weekly learning goals to successfully send #2102

Closed DeanJayMathew closed 1 year ago

DeanJayMathew commented 1 year ago

I just added more context to the instructions, clearly stating the five conditions that the script checks in order for an email to be sent or not.

Reviewers

Possible roles follow. The PR submitter checks the boxes after each reviewer finishes and gives :+1:.

Testing

Post-review

openedx-webhooks commented 1 year ago

Thanks for the pull request, @DeanJayMathew! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

feanil commented 1 year ago

@DeanJayMathew change looks great, let me know once you've signed the CLA.

sarina commented 1 year ago

@DeanJayMathew sorry I missed the original ping! Thanks for the addition.

@feanil I'm in Salesforce and Dean is covered by an Entity CLA. Let's figure this out offline.

sarina commented 1 year ago

@DeanJayMathew that was quick! I fixed our data (you are covered for CLA), but it'll take until tonight for the data to make its way to our automation.

In the meantime, one last thing you have to do is reword your commit message, in two ways: it needs to be more descriptive, and it needs to start with doc: (see https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0051-bp-conventional-commits.html)

How about, doc: Explain how to enable weekly learning goals?

I'm not sure how to do this from the GitHub GUI. If you'd like, I can make the change for you - just give me the final wording you'd like to use.

DeanJayMathew commented 1 year ago

Hi @sarina and @feanil, thanks!

I reworded it at the top of this page to be: doc: Explains the conditionals for weekly learning goals to successfully send

Have I done it correctly?

feanil commented 1 year ago

@DeanJayMathew unfortunately the PR title is not the same as the commit message. It's the commit message that needs to change. I've gone ahead and done that for you here. But if you're curious about how to do it, you can take a look at the docs here.

feanil commented 1 year ago

Also @DeanJayMathew does this change need to be ported to master as well?

feanil commented 1 year ago

Looks like they do, so I cherry-picked them to master in this PR: https://github.com/openedx/edx-documentation/pull/2103

openedx-webhooks commented 1 year ago

@DeanJayMathew 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

DeanJayMathew commented 1 year ago

Thank you @feanil !