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

feat: Add a config for openedx-events annotations #98

Merged
merged 5 commits into from
Mar 14, 2024

Conversation

bmtcril
Copy link
Contributor

@bmtcril bmtcril commented Feb 16, 2024

Description: Adding code and config to find and document annotations used in the openedx-events repository. When run in openedx-events (see openedx/openedx-events#318) the generated documentation looks like this:

Screenshot 2024-02-20 at 1 11 21 PM

@bmtcril bmtcril force-pushed the bmtcril/add_openedx_events branch 19 times, most recently from d4cf0e7 to 1718a0c Compare February 20, 2024 16:06
@bmtcril bmtcril force-pushed the bmtcril/add_openedx_events branch 5 times, most recently from 3185ec4 to 7bd4dca Compare February 20, 2024 17:58
@bmtcril bmtcril marked this pull request as ready for review February 20, 2024 18:40
Copy link
Contributor

@timmc-edx timmc-edx left a comment

Choose a reason for hiding this comment

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

My review shouldn't be considered authoritative as I'm not a maintainer and I'm not familiar with this repo, but -- I did review the fields/annotations for completeness and consistency.

@bmtcril
Copy link
Contributor Author

bmtcril commented Feb 20, 2024

@mariajgrimaldi thanks, the docs said that first argument was supposed to be a separator so I thought it would... do that. :) All set now.

Copy link
Member

@mariajgrimaldi mariajgrimaldi left a comment

Choose a reason for hiding this comment

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

@timmc-edx: this implementation is similar to the featuretoggles one

with implementation-specific changes to generate events docs instead. I understand you're not the maintainer of the repo, so could we include someone familiar with the code? Because of the similarity with the other extensions, I don't think it'd be difficult to review.

@timmc-edx
Copy link
Contributor

I've attempted to ping the owning team within 2U, but haven't heard back yet. I think if another week goes by without hearing from them, we should just merge this.

@timmc-edx
Copy link
Contributor

@bmtcril I think we're good to merge this.

@bmtcril bmtcril merged commit 767eeda into master Mar 14, 2024
9 checks passed
@bmtcril bmtcril deleted the bmtcril/add_openedx_events branch March 14, 2024 11:47
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.

3 participants