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

Typing fixes + docs improvements #121

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

Conversation

Jodenee
Copy link
Contributor

@Jodenee Jodenee commented Oct 3, 2024

This pr aims to correct typos found in the docs, update existing docstrings to be more consistent with other pages and fix some typing issues.

minor grammer change
minor grammer change
minor grammer change
typing fixes + improved wording
added periods to stay consistant with other objects
minor grammer changes
made set_token docstring a bit more clear
Copy link
Collaborator

@jmkd3v jmkd3v left a comment

Choose a reason for hiding this comment

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

A few changes. I left the comments on the remove side of the diff, not sure if that matters

roblox/assets.py Show resolved Hide resolved
product_id: Product id of the asset
name: Name of the Asset
description: Description of the Asset
type: Type of the Asset
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we shouldn't need to write "... of the Asset." over and over, although I don't know what exactly to put there instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could make it "The asset's ..." but I'm not really sure if that's better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the attribute descriptions to have less repetition is it better now?

roblox/assets.py Show resolved Hide resolved
roblox/assets.py Show resolved Hide resolved
@@ -44,16 +44,16 @@ class Badge(BaseBadge):
Represents a badge from the API.

Attributes:
id: The badge Id.
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it "a badge ID" or "a badge's ID"? i honestly don't know what's best, but considering we talk about "user IDs" a lot I'm leaning towards "The badge ID."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something about "The badge ID" doesn't really sit well with me. "The badge's ID" just sounds more clear and it's more consistent with other pages too. What do you think?

minor grammer change
reworded some stuff to be more clear + replace Id with ID for consistency
replace Id with ID for consistency
reworded some stuff to be more clear + replace Id with ID for consistency
reworded some stuff to be more clear + fixed incorrect attribute descriptions + replaced word game with universe
reworded some stuff to be more clear + replace Id with ID for consistency
@Jodenee
Copy link
Contributor Author

Jodenee commented Oct 8, 2024

While adding the requested changes I found more things to fix as well so maybe review again when you have the chance please.

@Jodenee Jodenee requested a review from jmkd3v October 10, 2024 19:52
Fixed warning admonition on get_instances
Reworded some attributes to be less repetitive
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.

2 participants