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

Add support for django 4.2 #359

Merged
merged 5 commits into from
Aug 4, 2023
Merged

Conversation

irtazaakram
Copy link
Contributor

Description

This is related to ongoing effort to update to Django 4.2 in openedx.
edx/upgrades#381

Supporting information

Changes:

problem_builder/tests/unit/test_answer_mixin.py

pylintrc

pylint: Command line or configuration file:1: UserWarning: Specifying exception names in the overgeneral-exceptions option without module name is deprecated and support for it will be removed in pylint 3.0. Use fully qualified name (maybe 'builtins.BaseException' ?) instead.

Can we also have a new release when this PR is merged?

Thanks,

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Patch coverage: 50.00% and no project coverage change.

Comparison is base (42c2d2f) 45.08% compared to head (c0cee35) 45.08%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #359   +/-   ##
=======================================
  Coverage   45.08%   45.08%           
=======================================
  Files          40       40           
  Lines        3309     3309           
  Branches      463      463           
=======================================
  Hits         1492     1492           
  Misses       1769     1769           
  Partials       48       48           
Flag Coverage Δ
unittests 45.08% <50.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
problem_builder/dashboard.py 24.26% <0.00%> (ø)
problem_builder/__init__.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, @irtazaakram!

Can we also have a new release when this PR is merged?

Please see this comment. I explained why we don't need to update the version in such a case. We mistakenly include the tests in the Python package in this repository, but we do not use them anywhere.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@irtazaakram
Copy link
Contributor Author

Hi @Agrendalath,

Thanks for the review. I've added integration test for Django 4.2. Let me know if this is the right approach.

Thanks,

Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

@irtazaakram, the approach looks good, but I'd like to understand where these additional changes (mentioned in the review comments) come from.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
problem_builder/dashboard.py Show resolved Hide resolved
requirements/base.in Outdated Show resolved Hide resolved
requirements/base.in Outdated Show resolved Hide resolved
Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this: checked that the CI uses Django 3.2 and 4.2.
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

@Agrendalath Agrendalath enabled auto-merge (squash) August 4, 2023 09:22
@Agrendalath Agrendalath merged commit b54b9e5 into open-craft:master Aug 4, 2023
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