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

Fixed Bug #111 + Enhancements #127

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed Bug #111 + Enhancements #127

wants to merge 2 commits into from

Conversation

Hrsj123
Copy link

@Hrsj123 Hrsj123 commented Dec 1, 2023

I have tried to fix the bug mentioned in Issue 111
I have also added an fixes to pass special characters (i.e. "#" in "C#") to the github api endpoint in a query parameter.

Checklist

  • My code is properly formatted using the latest black
  • I have added/updated tests
  • I have tried running my code manually
  • I have checked for spelling errors

Description

  • Bug Fix (resolves #111)
    • I have tackled the error caused by gtrending requests by creating a mapping variable which checks the language passed in by the user.
    • For example, in,
      -l csharp -d day
      The language "csharp" will be checked in the mapping values. It it is present in any list we replace the language by its key (i.e. c#).
    • PROBLEM: (Test case failure)
      assert repo["language"].lower() == language.lower()
      This assertion is in method "test_search_language" in "tests/test_search.py".
      This is because the response contains "c++" as language and the request of user comes with "cpp" as input,
  • Enhancements
    • I have also added a changes language query parameter which is passed to github api endpoint so it accepts special characters like "#" (Since special characters need to be encoded before it is passed in as query parameter)
    -   [f"+language:{language}" for language in languages]
    +  [f"+language:{requests.utils.quote(language)}" for language in languages]

Do let me know your thoughts on these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant