Closed allerter closed 3 years ago
The home page is a little short on content. The text of Scraping song lyrics from Genius.com 🎶 could help.
@Allerter, thank you so much for your work on this! I really like the additions you're making. I will make time this weekend to review your work.
Hi @Allerter, is the tox.ini
file included in your changes here? I have .ini
files excluded with gitignore
, so you may have unknowingly not added it.
I wasn't aware that .ini
files were excluded. The tox.ini
file contains tests for checking the codes against PEP8, checking the docs against PEP8, and checking to create the docs. It can come in handy when someone else wants to make a change in those files. But you can delete it if you want to.
Yes, I'm happy to include the tox.ini
file, but I don't think you've added it to your PR. Add it with a new commit, and I'll include it.
My bad, now I get what you meant. I'll add it in a bit.
Hey. The checks passed on my Travis CI account. Looks like you haven't set the GENIUS_CLIENT_ACCESS_TOKEN
in the environment variables of the repository on Travis CI's website.
I think I haven't given PRs permission to use my token on Travis. The tests pass in my local environment. I plan to merge your PR this weekend if not before then.
Why not give access when its value is secret? By the way, I tried to add you as a maintainer on readthedocs.io, but I think you don't have an account there (or your username is not the same as your GitHub username). If you'd like to be added, please let me know.
I added you on ReadTheDocs. You should have access now. I'd be glad to take part in maintaining the library and have sent you an email.
Documentation
Have a look at the docs here Considering #63 and the need for docs, I created documentation for the library using Sphinx and also used the RTD theme for it. I used the Google style for documenting the classes, and the functions. Since the library supports all Python 3 versions, I didn't use type hints to auto-document things in Sphinx and added them manually in the docstrings. I also added some examples under the methods, and some more are in the snippets page of the docs. I also added a picture to the index page for aesthetics. I didn't use the Genius logo, because I wasn't sure if using their logo here was allowed or no. This is the first version of the docs, so I'd appreciate any suggestions. To check out the built version of the docs for yourself, install the packages in the requirements file in the docs folder. And then run
make html
in the same directory (or just use the tox commands below).PEP8 Check
I also did the PEP8 checks, but I'm not sure if all of them contributed to the clarity and style of the code. So feel free to revert any of those. For PEP8, I used flake8 coupled with flake8-bugbear. As for the configs I set for flake8, they're all present in the tox.ini file.
The dreaded DIV class
Looks like Genius went and changed up the class of the lyrics' div tag again (#148). I provided two solutions in the mentioned issue, but two users reported that it didn't work for them. But I can't really say anything about that since the fixes both work for me. So I added the second fix to the code.
Tox
I added tox so you can check things yourself. Running
tox
in the source dir of the library will run flake8, doc8 (PEP8 for docs), and create the docs to see if they are created correctly. You can run the following tox commands for the checks:tox
- runs flake8, doc8, and tests creating docstox -e lint
- rung flake8 and doc8tox -e docs
tests creating docstox - test
runs the tests using pytest (needs GENIUS_CLIENT_ACCESS_TOKEN env var set)For tox, I had to edit setup.py, add some things to it, and remove reading the requirements from the text file. But if you don't want to keep the tox file, feel free to revert setup.py and remove the tox file.