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

Minor changes to consent_to_track & README.md #63

Closed
wants to merge 2 commits into from

Conversation

xfxf
Copy link

@xfxf xfxf commented Jul 24, 2018

  • consent_to_track parameter also accepts a nullable boolean
  • Added info to README.md about breaking API change

As per issue #62 - allowed consent_to_track parameter to be a boolean, and added info near the top of the README.md which should hopefully help people who upgrade their libraries and find that the API has broken.

I went to add tests, but notice the existing tests don't particularly lend theirselves to testing this (and there aren't any for consent_to_track functionality at the moment). I can add them if needed, but I manually tested this in Python 2.7, Python 3.4 & Python 3.6 and all works as expected.

xfxf added 2 commits July 24, 2018 17:30
 * ``consent_to_track`` parameter also accepts a nullable boolean
 * Added info to README.md about breaking API change
@coveralls
Copy link

coveralls commented Jul 24, 2018

Coverage Status

Coverage increased (+2.4%) to 98.806% when pulling 35ce786 on xfxf:master into 4bfe2fd on campaignmonitor:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 96.186% when pulling dc1bcce on xfxf:master into 4bfe2fd on campaignmonitor:master.

@xfxf
Copy link
Author

xfxf commented Jul 25, 2018

@katharosada I'm going to close this; with reflection this API change is confusing (it can now take 6 values, 3 of which are the same) and given this is a breaking change across all libraries, it's not a problem that is particularly unique to this implementation nor deserves to be at the top of the README.md.

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

2 participants