Remove Oxford Dictionaries API Functionality #210
Conversation
WayneLambert
added
dependencies
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey WayneLambert - Here's my review!
General suggestions:
- Ensure that all new or modified logic paths introduced by these changes are covered by existing or new tests.
- Verify that the removal of the Oxford API and its related components does not leave any functionality untested, especially the new logic replacing the Oxford API functionality.
- Confirm that any front-end components relying on data from the Oxford API are updated or removed to prevent runtime errors.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| @@ -72,7 +71,6 @@ def test_get_shortlisted_words(): | |||
| assert len(shortlisted_words) >= 1, "Should have at least one element" | |||
|
|
|||
|
|
|||
| @pytest.mark.vcr() | |||
There was a problem hiding this comment.
question (llm): I noticed that tests related to the Oxford Dictionaries API functionality have been removed, which aligns with the pull request's goal. However, it's important to ensure that the removal of these tests doesn't leave any functionality untested, especially the new logic replacing the Oxford API functionality. Could you confirm if all new or modified logic paths introduced by these changes are covered by existing or new tests?
| @@ -72,7 +71,6 @@ | |||
| assert len(shortlisted_words) >= 1, "Should have at least one element" | |||
|
|
|||
|
|
|||
| @pytest.mark.vcr() | |||
| def test_get_longest_possible_word(shortlisted_words: list): | |||
There was a problem hiding this comment.
praise (llm): Praise for updating the test to reflect the removal of the Oxford API dependency. This ensures that the test suite remains relevant and focused on the current functionality.
| @@ -40,23 +40,20 @@ def results_screen(request): | |||
| letters_chosen: str = request.GET["letters_chosen"] | |||
| players_word: str = request.GET["players_word"] | |||
|
|
|||
| valid_word = validations.is_in_oxford_api(players_word) | |||
There was a problem hiding this comment.
suggestion (llm): Removing the Oxford API call from the view is in line with the PR's objectives. However, it's crucial to ensure that the application's functionality remains consistent and that any new logic introduced to replace the API's functionality is adequately tested, particularly in edge cases where the previous implementation might have behaved differently.
| @@ -67,8 +64,7 @@ | |||
| "comp_word": comp_word, | |||
| "comp_word_len": len(comp_word) if comp_word else 0, | |||
| "comp_score": logic.get_game_score(len(comp_word)) if comp_word else 0, | |||
| "winning_word": comp_word if len(comp_word) > player_word_len else players_word, | |||
| "definition_data": definition_data, | |||
There was a problem hiding this comment.
suggestion (llm): The removal of definition data from the context is consistent with the removal of the Oxford API. It's good to see the cleanup in the views to reflect the removal of unused data. Ensure that any front-end components relying on this data are also updated accordingly to prevent runtime errors.
Addresses the following: