Skip to content

Conversation

@mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Mar 2, 2022

Description:
In this PR, a new event was introduced with its data classes, but it was missing two changes
that are being added here.

  • Add ordering field to DiscussionTopicContext
  • Add more context to class arguments

Testing instructions:
Follow test instructions in this PR.

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)

@JuanDavidBuitrago
Copy link
Contributor

I test and the result was great!!!

Screenshot from 2022-03-02 16-53-22

@felipemontoya
Copy link
Member

Given that this was something that broke the CI at edx-platorm and not here, I think we should be doing something to make the CI in this repo catch that kind of error. Is this a realistic possibility?

@mariajgrimaldi
Copy link
Member Author

mariajgrimaldi commented Mar 3, 2022

@felipemontoya we could try and maintain some tests here in sync. That would give us some reassurance on how the event is being used (the send_event with some dummy data). What do you think?

@mariajgrimaldi
Copy link
Member Author

I'll merge this PR and add a POC of tests for the events definitions.

* Add ordering field to DiscussionTopicContext
* Add more context to class arguments
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/fix-missing-discussion-field branch from 2466171 to cd016ac Compare March 3, 2022 15:29
@mariajgrimaldi mariajgrimaldi merged commit cd7fef6 into main Mar 3, 2022
@mariajgrimaldi mariajgrimaldi deleted the MJG/fix-missing-discussion-field branch August 31, 2022 20:28
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.

4 participants