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

VAN: Fix codes bugs. #696

Merged
merged 3 commits into from
Jun 7, 2022
Merged

VAN: Fix codes bugs. #696

merged 3 commits into from
Jun 7, 2022

Conversation

jburchard
Copy link
Collaborator

@jburchard jburchard commented Jun 1, 2022

Two small bug fixes:

  • get_code : The method for the underlying VANConnector class was misnamed.
  • apply_code: The is_applicable parameter was misnamed and thus, not applying.

Furthermore, I might want to rework the _supported_entities parameter. It requests constructing a dict, which is sort of unnecessary and not keeping with the common sense abstraction of Parsons. But, that would come in another PR.

@jburchard jburchard added the bug Impact - something is currently broken in Parsons and needs to be fixed label Jun 1, 2022
@jburchard jburchard requested a review from shaunagm June 1, 2022 18:01
@jburchard jburchard changed the title VAN: Fix get_code bug. VAN: Fix codes bugs. Jun 2, 2022
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

This looks good to me, but why wasn't it caught in the tests? Do we want to update the tests to cover this?

@jburchard
Copy link
Collaborator Author

Yes, yes. Good call. Someone (me) many years ago, probably ran out of steam when writing the tests. I'll take a look.

@jburchard
Copy link
Collaborator Author

Test added. Ready for re-review. @shaunagm

@shaunagm
Copy link
Collaborator

shaunagm commented Jun 7, 2022

Looks good to me. Thanks for adding the test, @jburchard!

@jburchard jburchard merged commit 8411c21 into main Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Impact - something is currently broken in Parsons and needs to be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants