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

Additional fix for GH-1071 (removing caps on banners). Banner parameters should be exported/pushed as objects. #81

Merged
merged 3 commits into from May 31, 2018

Conversation

@zarembsky
Copy link
Contributor

@zarembsky zarembsky commented May 25, 2018

  • [x ] Have you followed the guidelines in CONTRIBUTING.md?
  • [ x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • [x ] Have you added an explanation of what your changes do?
  • [x ] Does your submission pass tests?
  • [ x] Did you lint your code prior to submission?
@zarembsky zarembsky changed the base branch from master to develop May 25, 2018
@zarembsky
Copy link
Contributor Author

@zarembsky zarembsky commented May 25, 2018

Banner parameters should be exported/pushed as objects so as to not break pre-8.2 Ghostery versions out there and sync properly.

@christophertino christophertino requested a review from ghostery/ghostery as a code owner May 29, 2018
Copy link
Member

@christophertino christophertino left a comment

@zarembsky Should we also handle a similar case when pulling settings?

@zarembsky
Copy link
Contributor Author

@zarembsky zarembsky commented May 30, 2018

This case is already taken care of in src/classes/Conf.js, line 80. Handling data conversion in this place also takes care of importing settings from file, which goes through invoking PanelData.set, which eventually invokes conf set trap.

@christophertino christophertino merged commit 709ffea into develop May 31, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@zarembsky zarembsky deleted the feature/Banners branch Jun 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants