Port [StackExchange] from legacy to BaseJsonService #2418
Conversation
Per part of issue badges#2378, convert StackExchange service to the newer BaseJson class. Refactor also to seperate StackExchange reputation and questions tag info into new StackExchange subservice files as the legacy class contained both which may be difficult to keep track of in the future as the service continues to grow. More tests have been added to make sure other StackExchange sites besides StackOverflow will still work with the new badge setup. If merged, next step would be to add a new StackOverflow badge part of badges#2378.
|
I deployed a review app: http://shields-staging-pr-2418.herokuapp.com/ Something's not quite right with the examples:
|
|
Thanks for all the feedback you gave @paulmelnikow 👍 It's really helpful especially for someone who is starting to contribute to open source |
|
Nice work! 🏆 And please keep it up! There's so much to do in this project and I and the other maintainers are happy to help you find more work that interests you. |
|
This is deployed to s0 and it looks like it's not working quite right: http://s0.shields-server.com/stackexchange/stackoverflow/r/893113.svg This is the old code: http://s1.shields-server.com/stackexchange/stackoverflow/r/893113.svg Do you mind taking a look? |
|
Interesting so the new link you gave should be now routed to instead but I'm not sure why its returning invalid instead of the data we need. |
|
I'm not exactly sure what you mean. However, we should make sure the URL pattern that worked in the past also work going forward. |
|
Right, I'll need to rework these files then to use the past URL pattern. My apologies, I'll work on another PR for this 👍 |
|
Thanks! My bad for not noticing it during the review! The tests moved from one file to another and I didn't realize the routes changed. I'm rolling back to 1a75790 until this is fixed. |
|
@bui1 Is this something you could do in the next day? It would be great to get other work shipped and this is a deploy blocker. If not, zero pressure, I could take in on. |
…#2418 Url patterns have been changed back to their older legacy format. Tests now run properly with this URL in addition to the examples showing up on local.
…2418 (#2430) * Fix URL pattern for StackExchange Questions and Reputation per #2418 Url patterns have been changed back to their older legacy format. Tests now run properly with this URL in addition to the examples showing up on local. * Make changes per review comments. Also change "q" to "t" parameter to match legacy url for questions
StackOverflow will still work with the new badge setup.
If merged, next step would be to add a new StackOverflow badge part of #2378.