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

changes as per PEP-8 and MD001 #134

Merged
merged 9 commits into from Nov 13, 2021

Conversation

Siddhesh-Agarwal
Copy link
Contributor

Heading levels should only increment by one level at a time [Expected: h2; Actual: h3](https://github.com/DavidAnson/markdownlint/blob/v0.24.0/doc/Rules.md#md001)
Heading levels should only increment by one level at a time [Expected: h2; Actual: h3](https://github.com/DavidAnson/markdownlint/blob/v0.24.0/doc/Rules.md#md001)
Changed function name from playonyt()` to `play_on_yt()` in order to comply with [PEP 8](https://www.python.org/dev/peps/pep-0008/#function-and-variable-names)
@Siddhesh-Agarwal Siddhesh-Agarwal marked this pull request as ready for review October 19, 2021 07:32
@aaryanrr aaryanrr added the documentation Improvements or additions to documentation label Oct 21, 2021
@aaryanrr
Copy link
Collaborator

Hey @Siddhesh-Agarwal , thanks for your valuable contribution. We'll review this PR soon and you'll will be updated accordingly.

f-strings are
1. more readable and aesthetically pleasing
2. easier to implement than `+` and `.format()` string formatting styles. 

Furthermore, using f-strings is suggested for Python 3.6 and above while `.format()` is best suited for Python 2.6 and above. Versions prior to Python 2.6 only provide % option for string formatting.
Changed default color to (0, 0, 0)
Changed string formatting style
@@ -45,13 +45,11 @@ def info(topic: str, lines: int = 3, return_value: bool = False) -> Optional[str
return data


def playonyt(topic: str, use_api: bool = False, open_video: bool = True) -> Union[str]:
def play_on_yt(topic: str, use_api: bool = False, open_video: bool = True) -> Union[str]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Renaming this function will break the codes used by others so its better to not change it

@aaryanrr aaryanrr added this to In progress in Next Version via automation Nov 13, 2021
@aaryanrr
Copy link
Collaborator

Thanks for your valuable contribution @Siddhesh-Agarwal :)

@aaryanrr aaryanrr merged commit 52b76cf into Ankit404butfound:master Nov 13, 2021
Next Version automation moved this from In progress to Done Nov 13, 2021
@aaryanrr aaryanrr removed this from Done in Next Version Nov 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants