Sync upstream with no conflict #185
Conversation
…event.0034_fix_language_codes (#1789)
overrides #1792
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Reviewer's Guide by SourceryThis pull request contains a variety of changes across multiple files, including bug fixes, performance improvements, code style enhancements, and feature additions. Key changes include:
File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @it-tma-tri - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -24,7 +24,7 @@ def __init__(self, *args, **kwargs): | |||
| kwargs["locales"] = getattr(self.event, "locales", []) | |||
| super().__init__(*args, **kwargs) | |||
|
|
|||
| def _construct_form(self, i, **kwargs): | |||
| def _construct_form(self, *args, **kwargs): | |||
There was a problem hiding this comment.
suggestion (performance): Potential performance improvement in form handling
By changing the method signature to use *args, you've made the method more flexible and potentially more efficient, especially when handling a large number of forms.
| @@ -386,7 +386,7 @@ def test_schedule_export_schedule_html_task(mocker, event, slot): | |||
| call_command, | |||
| ) | |||
There was a problem hiding this comment.
suggestion (testing): Update assertion for ignore_result parameter
The test for the orga trigger export has been updated to include ignore_result=True. Consider adding an assertion to verify that this parameter is being passed correctly in the apply_async call.
| ) | |
| call_command, | |
| ) | |
| assert 'ignore_result' in export_schedule_html.apply_async.call_args[1] | |
| assert export_schedule_html.apply_async.call_args[1]['ignore_result'] is True |
| ], | ||
| } | ||
| try: | ||
| response = requests.post( | ||
| "https://pretalx.com/.update_check/", | ||
| json=check_payload, | ||
| timeout=30, |
There was a problem hiding this comment.
issue (complexity): Consider reviewing the introduction of new parameters.
The recent changes to the code are well-considered and do not significantly increase complexity. The addition of the name parameter in the task decorator adds specificity, which can be useful if the task name is referenced elsewhere. Including a timeout in the requests.post call is a good practice to prevent indefinite hanging, though it introduces an additional parameter to manage. The change from p to plugin in the list comprehension enhances readability, making the code more self-explanatory. Lastly, the ignore_result=True parameter simplifies task result handling when the result is not needed, though it introduces a new concept to understand. Overall, these changes improve the code's robustness and clarity.
|
Tests are failing and NameError: name 'HttpRequest' is not defined |
|
Hi @mariobehling, conflict solved and tests passed as well |
Summary by Sourcery
Sync the upstream repository with various improvements, including a new feature for dynamic tab title updates in the CfP wizard, multiple bug fixes, enhancements to task handling and HTML export, and updates to translations and documentation. Additionally, update the build configuration and improve test coverage.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: