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

Declare OEP-7 compliance #104

Merged
merged 2 commits into from May 28, 2019
Merged

Declare OEP-7 compliance #104

merged 2 commits into from May 28, 2019

Conversation

jmbowman
Copy link
Contributor

@jmbowman jmbowman commented May 15, 2019

  • Test Python 2.7 in Travis again
  • Switch from testing Python 3.5 to 3.6
  • Drop support for testing against Django 1.8
  • Update requirements
  • Fix new pylint warnings
  • Declare OEP-7 support
  • Update ownership metadata

@jmbowman jmbowman requested a review from cpennington May 15, 2019 18:46
@jmbowman
Copy link
Contributor Author

Closing and reopening to trigger a Travis CI build (it wasn't configured correctly).

@coveralls
Copy link

coveralls commented May 15, 2019

Coverage Status

Coverage remained the same at 96.342% when pulling bc29ddf on jmbowman/OEP-7 into 9807168 on master.

oeps: {}
owner: edx/platform-team
oeps:
oep-7: true
Copy link
Contributor

Choose a reason for hiding this comment

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

also, oep-2 and oep-18 compliant?

URL_RE_SOURCE = r"""
(?P<org>{ALLOWED_ID_CHARS}+)\+(?P<course>{ALLOWED_ID_CHARS}+)\+(?P<run>{ALLOWED_ID_CHARS}+)\+
(?P<block_type>{ALLOWED_ID_CHARS}+)\+
URL_RE_SOURCE = u"""
Copy link
Contributor

Choose a reason for hiding this comment

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

There are backslashes in the ALLOWED_ID_CHARS var, do those need to be updated as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the r at the start of a string literal only affects how characters shown explicitly in the source code for that string literal are treated. (The combined ur prefix no longer exists in Python 3, hence why we need the escaping here).

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, so the contents of the var will be escaped correctly before the strings are combined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, its content is independent of the prefixes used for the string literal referencing it.

@jmbowman jmbowman requested a review from feanil May 24, 2019 20:24
@jmbowman jmbowman merged commit 2967eb8 into master May 28, 2019
@jmbowman jmbowman deleted the jmbowman/OEP-7 branch May 28, 2019 13:47
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.

None yet

3 participants