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

Rewrite Reference and ReferenceList fields on import #2543

Merged
merged 1 commit into from
Feb 12, 2014

Conversation

cahrens
Copy link

@cahrens cahrens commented Feb 10, 2014

Fixes STUD-149.

@dmitchell @singingwolfboy Please review. This branch is based off of #2503, so the first commit can be ignored (assuming you reviewed the other one!).

@cahrens
Copy link
Author

cahrens commented Feb 10, 2014

Diffcover is lying-- the "Reference" code is covered by test_rewrite_reference (verified with breakpoint in debugger).

tag=target_location_namespace.tag,
org=target_location_namespace.org,
course=target_location_namespace.course
).url()
Copy link
Contributor

Choose a reason for hiding this comment

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

The spacing makes this hard to read. I'd format this more like:

if (original_location.tag == ref.tag and 
        original_location.org == ref.org and
        original_location.course == ref.course):
    new_ref = ref.replace(
        tag=target_location_namespace.tag,
        org=target_location_namespace.org,
        course=target_location_namespace.course
    ).url()

or

def locations_equal(a, b):
    return (a.tag == b.tag and a.org == b.org and a.course == b.course)

if locations_equal(original_location, ref):
    new_ref = ref.replace(
        tag=target_location_namespace.tag,
        org=target_location_namespace.org,
        course=target_location_namespace.course
    ).url()

@singingwolfboy
Copy link
Contributor

Again, only nitpicks, so 👍. As for diffcover, you should probably file a bug report with @wedaly.

@dmitchell
Copy link
Contributor

👍

cahrens pushed a commit that referenced this pull request Feb 12, 2014
Rewrite Reference and ReferenceList fields on import
@cahrens cahrens merged commit b0b8257 into master Feb 12, 2014
@cahrens cahrens deleted the christina/reference_bug branch February 12, 2014 21:07
@cahrens
Copy link
Author

cahrens commented Feb 12, 2014

Filed https://edx-wiki.atlassian.net/browse/TE-346 for diffcover bug.

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.

3 participants