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

Modify the return value of session_type #207

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

odkhang
Copy link
Collaborator

@odkhang odkhang commented Sep 27, 2024

This PR support filter by user language in video component make by this PR: fossasia/eventyay-video#239
return object with multiple language instead of string value.

How has this been tested?

Checklist

  • I have added tests to cover my changes.

Summary by Sourcery

Update the 'session_type' return value in the schedule model to return only the submission type name, simplifying the data structure.

Enhancements:

  • Modify the return value of 'session_type' to return the name of the submission type instead of a string with the name and duration.

Copy link

sourcery-ai bot commented Sep 27, 2024

Reviewer's Guide by Sourcery

This pull request modifies the build_data method in the schedule/models/schedule.py file. The main change is to simplify the session_type field in the returned data structure. Instead of returning a string that combines the submission type name and duration, it now returns only the submission type name.

No sequence diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Simplification of the session_type field in the build_data method
  • Removed the concatenation of submission type name and duration
  • Now only returns the submission type name
  • Eliminated the string formatting for duration
src/pretalx/schedule/models/schedule.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @odkhang - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider adding a brief explanation in the PR description about why returning just the name of the submission type is preferable to the previous format that included duration.
  • The PR checklist mentions adding tests, but none seem to be included. Please add appropriate tests to cover this change or update the checklist if tests are not necessary.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@@ -676,12 +676,8 @@ def build_data(self, all_talks=False, filter_updated=None, all_rooms=False):
),
"do_not_record": talk.submission.do_not_record,
"tags": talk.submission.get_tag(),
Copy link

Choose a reason for hiding this comment

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

suggestion: Clarify the naming of the 'tags' field

The 'tags' field is populated by a method called 'get_tag()' (singular). This mismatch between plural and singular forms could be confusing. Consider either renaming the field to 'tag' if it's always a single tag, or ensure that 'get_tag()' returns a list of tags if multiple tags are possible.

                        "tags": talk.submission.get_tags(),

@mariobehling mariobehling merged commit 192d589 into fossasia:development Sep 29, 2024
7 of 8 checks passed
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.

2 participants