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-1537 : Upgrade to Python 3.8 #452

Merged
merged 3 commits into from
May 1, 2020
Merged

Conversation

mzulqarnain1
Copy link

@mzulqarnain1 mzulqarnain1 commented Apr 20, 2020

Relevant JIRA : https://openedx.atlassian.net/browse/BOM-1537

  • Added Python 3.8 to testing
  • Removed Django versions < 2.2
  • Ran make upgrade
  • Removed constraints no longer needed
  • Fixed new quality failures

@tirkarthi
Copy link

@mzulqarnain1 See also #454 #453 for some potential warnings related to migration.

@mzulqarnain1 mzulqarnain1 force-pushed the zulqarnain/BOM-1537 branch 5 times, most recently from bbbe558 to 734bca2 Compare April 24, 2020 09:10
@mzulqarnain1 mzulqarnain1 marked this pull request as ready for review April 24, 2020 09:17
@mzulqarnain1 mzulqarnain1 requested review from jmbowman, awais786 and UsamaSadiq and removed request for jmbowman April 24, 2020 09:17
pylintrc Outdated Show resolved Hide resolved
@@ -146,15 +146,15 @@ def POST(self):
querydict_to_multidict(self._request.FILES, wrap=DjangoUploadedFile),
)

@lazy
def body(self): # pylint: disable=method-hidden
@property
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a reason for this change.

Copy link
Author

Choose a reason for hiding this comment

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

Both of these are marked as property in class we are inheriting from so pylint suggested making it property in child class too.

@@ -139,7 +138,7 @@ def load_classes(cls, fail_silently=True):
"""
all_classes = itertools.chain(
pkg_resources.iter_entry_points(cls.entry_point),
(entry_point for identifier, entry_point in cls.extra_entry_points),
(entry_point for identifier, entry_point in iter(cls.extra_entry_points)),
Copy link
Contributor

Choose a reason for hiding this comment

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

why you are doing this ?

Copy link
Author

Choose a reason for hiding this comment

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

This is a lazy member of our class which returns a list when called, but pylint doesn't consider it an iterable and warns, so wrapping it in iter() does the trick.

@@ -54,22 +54,18 @@ def __new__(cls, scope, user_id, block_scope_id, field_name, block_family='xbloc
@abstractmethod
def get(self, key):
"""Reads the value of the given `key` from storage."""
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can disable the pass warning instead of removing.

Copy link
Author

Choose a reason for hiding this comment

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

Should I go on and re-add all the pass?

tox.ini Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
@mzulqarnain1 mzulqarnain1 merged commit a8b00d4 into master May 1, 2020
@mzulqarnain1 mzulqarnain1 deleted the zulqarnain/BOM-1537 branch May 1, 2020 09:31
@stvstnfrd stvstnfrd mentioned this pull request Oct 20, 2020
1 task
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.

5 participants