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

Use gtrending for fetching GitHub trending instead of scraping the web #86

Merged
merged 12 commits into from
Sep 13, 2020

Conversation

odmishien
Copy link
Contributor

@odmishien odmishien commented Sep 1, 2020

resolves #82

Checklist

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

Description

  • Use gtrending for fetching GitHub trending instead of scraping the web
  • Define convert_gtrending_repo_to_repo that convert gtrending_repo dict to github uses one
  • Remove bs4 & lxml from requirements.txt and setup.py
  • Format files by black command

@odmishien odmishien marked this pull request as draft September 1, 2020 05:26
@odmishien odmishien marked this pull request as ready for review September 1, 2020 05:36
starcli/search.py Outdated Show resolved Hide resolved
starcli/search.py Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@odmishien
Copy link
Contributor Author

odmishien commented Sep 7, 2020

@hedythedev Thank you for your review! I fixed it.
And I can't understand why test was failed, because I couldn't find out the test assert 0 == 1 in our code. Would you know what's going on?

=================================== FAILURES ===================================
______________________________ test_search_stars _______________________________

    def test_search_stars():
        """
        Test the search functionality for starcli.search.
        """
        repos = search(language="python", stars="1")
        for repo in repos:
>           assert repo["stargazers_count"] == 1
E           assert 0 == 1

tests/test_search.py:114: AssertionError

@odmishien odmishien requested a review from hedyhli September 8, 2020 04:32
Copy link
Owner

@hedyhli hedyhli left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, it seems that the test only fails in some cases. Currently it does not fail at all for me locally, will keep looking for where the problem is

starcli/search.py Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
starcli/search.py Outdated Show resolved Hide resolved
@odmishien
Copy link
Contributor Author

@hedythedev Thank you for your review! I fixed it 🔧

@odmishien odmishien requested a review from hedyhli September 9, 2020 03:36
Copy link
Owner

@hedyhli hedyhli left a comment

Choose a reason for hiding this comment

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

It should have been .replace("-", " ") not .replace("-", "") (the space), but no worries I will fix it, thanks for your contributions!

hedyhli added a commit that referenced this pull request Sep 13, 2020
@hedyhli hedyhli merged commit be834be into hedyhli:main Sep 13, 2020
@odmishien odmishien deleted the feature-82/use-gtrending branch September 14, 2020 12:05
@hedyhli
Copy link
Owner

hedyhli commented Oct 2, 2020

@allcontributors please add @odmishien for code

@allcontributors
Copy link
Contributor

@hedythedev

I've put up a pull request to add @odmishien! 🎉

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.

Use gtrending for fetching GitHub trending instead of scraping the web
2 participants