Skip to content

docs: adr for enabling producing to event bus via settings - #224

Merged
robrap merged 1 commit into
openedx:mainfrom
open-craft:navin/configurable-handler-adr
Jun 26, 2023
Merged

docs: adr for enabling producing to event bus via settings#224
robrap merged 1 commit into
openedx:mainfrom
open-craft:navin/configurable-handler-adr

Conversation

@navinkarkera

@navinkarkera navinkarkera commented May 23, 2023

Copy link
Copy Markdown
Contributor

Description: ADR for enabling producing to event bus via settings

ISSUE: #210

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

@navinkarkera
navinkarkera requested a review from a team May 23, 2023 06:29
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @navinkarkera! 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:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

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.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 23, 2023
@timmc-edx timmc-edx self-assigned this May 23, 2023
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
@navinkarkera
navinkarkera force-pushed the navin/configurable-handler-adr branch from 42568a5 to ca804fc Compare May 26, 2023 06:10
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
@navinkarkera
navinkarkera force-pushed the navin/configurable-handler-adr branch from ca804fc to 4db9565 Compare May 31, 2023 09:55

@tecoholic tecoholic left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@navinkarkera 👍 LGTM

  • I tested this: NA
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@mphilbrick211

Copy link
Copy Markdown

@mariajgrimaldi would you be able to review/merge this?

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm excited about this change. Thanks for the ADR work.

Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
@robrap
robrap self-requested a review June 1, 2023 20:55
@navinkarkera
navinkarkera force-pushed the navin/configurable-handler-adr branch from 4db9565 to 2f4dbed Compare June 5, 2023 06:48
@navinkarkera
navinkarkera requested a review from timmc-edx June 5, 2023 06:53

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll have someone else join in on the discussion of where to define defaults. Thank you.

Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
@navinkarkera
navinkarkera force-pushed the navin/configurable-handler-adr branch from b1d9611 to fd624f9 Compare June 6, 2023 05:33
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
@navinkarkera
navinkarkera requested review from robrap and timmc-edx June 7, 2023 06:51
@sameenfatima78

Copy link
Copy Markdown
Member

@mariajgrimaldi @mphilbrick211 @robrap Hi folks, would it be possible to get this merged soon? It will really help with the deployment of our project.

@robrap

robrap commented Jun 9, 2023

Copy link
Copy Markdown
Contributor

@sameenfatima78: Navin will need to speak to the timeline. At the same time, temporarily adding the hardcoded version with a feature flag shouldn’t be a big deal and will keep you unblocked. Let me know if you want to discuss further.

Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/index.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0011-producing-to-event-bus-via-settings.rst

@mariajgrimaldi mariajgrimaldi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I left a few tiny comments

Comment thread docs/decisions/0012-producing-to-event-bus-via-settings.rst Outdated
Comment thread docs/decisions/0012-producing-to-event-bus-via-settings.rst Outdated
@navinkarkera
navinkarkera force-pushed the navin/configurable-handler-adr branch from d96b3d1 to 17c7dd3 Compare June 26, 2023 05:29
@navinkarkera

Copy link
Copy Markdown
Contributor Author

This is ready to be merged now.

@robrap
robrap merged commit 37a84a7 into openedx:main Jun 26, 2023
@openedx-webhooks

Copy link
Copy Markdown

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

@navinkarkera
navinkarkera deleted the navin/configurable-handler-adr branch June 26, 2023 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

9 participants