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

Adding region selection to ReutersBridge #10

Merged
merged 28 commits into from
Jan 4, 2021

Conversation

Spraynard
Copy link

Adding in region selection which in essence changes the article_uri parameter. This parameter is used to effectively link back to the region specific Reuters website that contains the article.

hollowleviathan and others added 12 commits July 10, 2020 08:34
News Feed types now selectable through a list parameter

JSON return is now being parsed correctly

Stories being gathered in a stepwise process
News Feed types now selectable through a list parameter

JSON return is now being parsed correctly

Stories being gathered in a stepwise process
[ReutersBridge] Add all article from 'Editor\'s Highlight' to the feed, more categories, author name, full article text.
Here're some new change to Reuters Bridge:

    Remove Pic of the Day section due a lot of images, might not suitable for a RSS feed,
    Modifying function getJson() for mose reusable.
    Add some news sections.
    Fix PHP warning about undeclared index, trying to access null point, etc,......
    Add categories.
    Caption included for each image and move all of them down to the bottom part of the article.
Adding in Feed region selector

Adding in new class constants for feed region selection
@csisoap
Copy link

csisoap commented Dec 28, 2020

Some API link may not work with the UK edition. Example:

https://wireapi.reuters.com/v8/feed/rapp/us/tabbar/feeds/health <-- Worked with U.S edition

https://wireapi.reuters.com/v8/feed/rapp/uk/tabbar/feeds/health <-- This is not working

EDIT:
I've just tested all link included with the bridge and noted most of the links worked with U.K edition, except Health (as i mention above), U.S News, China.

@Spraynard
Copy link
Author

Awesome thank you for testing!

I see what you mean. I don't know if there is a simple fix for coding past that, to get a dynamic selection for available feeds based on region... Seemingly some JS would have to be introduced.

@csisoap
Copy link

csisoap commented Dec 28, 2020

I've just thinked about making each region its own context. For example, like Arte +7 Bridge here:
image

@Spraynard
Copy link
Author

Oh shoot, you can do that? Dang I will definitely check out that bridge

csisoap and others added 10 commits December 29, 2020 21:37
- Each edition of Reuters now splited up into each context.
- Rewrite some code logic, now using their type in API to checking.
- Add some new feeds specific to U.K edition.
Adding UK context and US context
Removing custom functions from class to pass tests
@Spraynard
Copy link
Author

This new class should implement a contextual view to differentiate regions.

Feeds that were not available for the UK region have been taken out.

@csisoap
Copy link

csisoap commented Dec 30, 2020

This new class should implement a contextual view to differentiate regions.

Feeds that were not available for the UK region have been taken out.

Hi @Spraynard , I've added some new feed specific to U.K edition and make some change to ReutersBridge. You may to check it out right here: Spraynard#2.

Sorry if i making a mess for you, i'm still noob to GitHub.

@Spraynard
Copy link
Author

Hey @hollowleviathan thank you for the changes and adding the "Editors Highlight" section to the bridge, it looks nice 🎉 .

I would also recommend to not force push with git unless necessary, as it will cause other developers to have to resolve merge conflicts if they want to work on it as well. If you need help interacting with git a little more then you can def ask me for help.

@Spraynard
Copy link
Author

Alright I think that we are at a point with this pull request where it can be merged. Let me know if there are any issues.

@Spraynard
Copy link
Author

Hello! Just wondering if there is anything stopping this commit from being merged? Thanks

@hollowleviathan
Copy link
Owner

Sorry about that, the delay was from the holiday weekend.

@hollowleviathan hollowleviathan merged commit fc81273 into hollowleviathan:reuters Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants