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

BOM-1015 #20

Merged
merged 1 commit into from
Nov 21, 2019
Merged

BOM-1015 #20

merged 1 commit into from
Nov 21, 2019

Conversation

awais786
Copy link
Contributor

@awais786 awais786 commented Nov 6, 2019

Adding tox for django 2, 2.1, 2.2 under python3.5

Description: Describe in a couple of sentence what this PR adds

JIRA: Link to JIRA ticket
https://openedx.atlassian.net/browse/BOM-1015

Dependencies: dependencies on other outstanding PRs, issues, etc.

Merge deadline: List merge deadline (if any)

Installation instructions: List any non-trivial installation
instructions.

Testing instructions:

  1. Open page A
  2. Do thing B
  3. Expect C to happen
  4. If D happened instead - check failed.

Reviewers:

  • tag reviewer
  • tag reviewer

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed
  • PR author is listed in AUTHORS

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)

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

@@ -58,7 +58,7 @@ class ContentDate(models.Model):
"""

course_id = CourseKeyField(db_index=True, max_length=255)
policy = models.ForeignKey(DatePolicy)
policy = models.ForeignKey(DatePolicy, on_delete=models.CASCADE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

django2 requirement

@awais786 awais786 requested a review from jmbowman November 6, 2019 11:55
django-waffle==0.17.0 # via edx-django-utils, edx-drf-extensions
django==1.11.24
django-waffle==0.18.0 # via edx-django-utils, edx-drf-extensions
django==1.11.26 ; python_version < "3"
Copy link

Choose a reason for hiding this comment

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

This isn't correct, so we may need to give up on the upper constraint on Django in base.in and just leave it as Django>=1.11; I don't think there's a good way to get pip-compile to generate what we want here with an environment marker involved (there's an open issue for this). I've updated the BOM-1009 description accordingly.

tox.ini Outdated
@@ -37,6 +37,8 @@ norecursedirs = .* docs requirements
deps =
django111: Django>=1.11,<2.0
django20: Django>=2.0,<2.1
django21: Django>=2.1,<2.2
django22: Django>=2.2,<2.3
-r{toxinidir}/requirements/test.txt
commands =
py.test {posargs}
Copy link

Choose a reason for hiding this comment

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

To output any applicable deprecation warnings, this needs to be updated to python -Wd -m pytest {posargs}.

@awais786
Copy link
Contributor Author

awais786 commented Nov 6, 2019

@jmbowman Please check again.

@awais786
Copy link
Contributor Author

awais786 commented Nov 6, 2019

@jmbowman
Copy link

jmbowman commented Nov 6, 2019

That warning is originating in the edx-opaque-keys package, so it'll need to be updated also. I just created BOM-1017 for that.

@awais786
Copy link
Contributor Author

awais786 commented Nov 8, 2019

This PR depends on this PR for warnings fixes openedx/opaque-keys#111

@awais786 awais786 force-pushed the awais786/BOM-1015 branch 3 times, most recently from b555d65 to 5ca9f2c Compare November 18, 2019 09:15
@awais786
Copy link
Contributor Author

@jmbowman I have waited for opaque-key PR to resolve the warnings but I think its fine to merge this now and update opaque-key version in next PR after resolving the warnings.
What do you think?

@awais786
Copy link
Contributor Author

@jmbowman I have waited for opaque-key PR to resolve the warnings but I think its fine to merge this now and update opaque-key version in next PR after resolving the warnings.
What do you think?

@jmbowman any update?

@jmbowman
Copy link

Sure, that works for me.

Adding tox for django 2, 2.1, 2.2 under python3.5
@awais786 awais786 merged commit a2c2d9a into master Nov 21, 2019
@awais786 awais786 deleted the awais786/BOM-1015 branch November 21, 2019 05:55
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