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

[backport] [scraper] Pass JSON serialized path settings to python scrapers #17112

Merged
merged 3 commits into from Jan 10, 2020

Conversation

Montellese
Copy link
Member

Description

This is a partial backport of #17062 and #17063.

Motivation and Context

It was requested by @romanvm and @rmrector.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@Montellese Montellese added Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality Wiki: Needed Component: Settings Component: Add-ons v18 Leia Type: Backport labels Jan 1, 2020
@Montellese Montellese added this to the Leia 18.6 milestone Jan 1, 2020
@Montellese Montellese added this to In progress in General (roadmap) via automation Jan 1, 2020
General (roadmap) automation moved this from In progress to Reviewer approved Jan 2, 2020
@Montellese Montellese force-pushed the backport/settings_json_serializer branch 2 times, most recently from 056bc08 to d4ffa2b Compare January 2, 2020 11:34
@Montellese
Copy link
Member Author

IMO this is ready to be merged into Leia. Who is the release manager?

@rmrector
Copy link
Contributor

rmrector commented Jan 3, 2020

@DaveTBlake

@DaveTBlake
Copy link
Member

Sorry I have not been following this work. I'm sure it is all fine, but can someone just confirm the scope of the impact of this change. I assume Python scrapers only, is that correct?

@rmrector
Copy link
Contributor

rmrector commented Jan 4, 2020

Ya, the functional change is limited to Python scrapers. There is also a structural change for subsettings, motivation detailed in the second paragraph of #17062 .

@Montellese
Copy link
Member Author

Actually I think this would even work without refactoring where ISubSettings are handled. I'll give it a try and report back. Without that commit this would purely touch the python scraper implementation.

@Montellese Montellese force-pushed the backport/settings_json_serializer branch from d4ffa2b to 1571fa7 Compare January 4, 2020 19:06
@Montellese
Copy link
Member Author

Yup that seems to work as well. @rmrector could you verify the functionality again with the reduced version?

Copy link
Contributor

@rmrector rmrector left a comment

Choose a reason for hiding this comment

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

Yup, a quick run works just fine. Thanks for slimming down the backport.

@Montellese
Copy link
Member Author

@DaveTBlake now it's really ready :-)

@DaveTBlake
Copy link
Member

Thanks for slimming this down @Montellese , always good to simplify backports if possible, glad that I was slow on the button.

@DaveTBlake DaveTBlake merged commit b2a38d4 into xbmc:Leia Jan 10, 2020
General (roadmap) automation moved this from Reviewer approved to Done Jan 10, 2020
@Montellese Montellese deleted the backport/settings_json_serializer branch January 11, 2020 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Add-ons Component: Settings Type: Backport Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality v18 Leia Wiki: Needed
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants