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.

Sign up for GitHub

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

Jump to bottom

fix: typos in models #164

Merged
merged 8 commits into from
Aug 25, 2023
Merged

fix: typos in models #164

merged 8 commits into from
Aug 25, 2023

Conversation

EnricoBaivo
Copy link
Contributor

@EnricoBaivo EnricoBaivo commented Aug 24, 2023 •
edited
Loading

event.py Optional[Achievement] = None # typo achievment
common.py Achievement(BaseModel)

  • description: str # typo desciption
  • mode: Optional[Gamemode] = Null

Example for Achievement that has a null as gamemode

"achievement": {
            "id": 319,
            "name": "Autocreation",
            "slug": "all-secret-autocreation",
            "description": "Absolute rule.",
            "grouping": "Hush-Hush",
            "icon_url": "https://assets.ppy.sh/medals/web/all-secret-autocreation.png",
            "mode": null,
            "ordering": 2,
            "instructions": "<i>One is all.</i>"
}

Self-check

  • The changes are tested with pre-commit
  • The changes pass unit testing with pytest
  • The changes pass mypy

@EnricoBaivo
Changed: event.py Fixed typo errors in Event(BaseModel) achievement: …
e391d95 
…Optional[Achievement] = None # typo achievment and in common.py Achievement(BaseModel) description: str # typo desciption
pre-commit-ci bot and others added 2 commits August 24, 2023 20:01
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
2ad8cbb 
for more information, see https://pre-commit.ci
@EnricoBaivo
Fixed: Achievement class parameter mode is a optional Field, possible…
c50b394 
… can be null. Example is the achievement: Autocreation that has a gamemode of null
NiceAesth
NiceAesth requested changes Aug 24, 2023
View reviewed changes
aiosu/models/event.py Outdated
@@ -65,7 +65,7 @@ class Event(BaseModel):
type: EventType
r"""Information on types: https://github.com/ppy/osu-web/blob/master/resources/assets/lib/interfaces/event-json.ts"""
parse_error: Optional[bool] = None
achievment: Optional[Achievement] = None
achievement: Optional[Achievement] = None # typo achievment
Copy link
Owner

Choose a reason for hiding this comment

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

Please do not add comments like these to the code

aiosu/models/common.py Outdated
@@ -46,10 +46,10 @@ class Achievement(BaseModel):
id: int
name: str
slug: str
desciption: str
description: str # typo desciption
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

.pre-commit-config.yaml Outdated
@@ -42,4 +42,4 @@ repos:
- id: poetry-check

default_language_version:
python: python3.9
python: python3.11
Copy link
Owner

Choose a reason for hiding this comment

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

Do not do unrelated changes in prs

.gitignore Outdated
@@ -26,7 +26,8 @@ share/python-wheels/
.installed.cfg
*.egg
MANIFEST

.gitigonre
Copy link
Owner

Choose a reason for hiding this comment

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

Do not do unrelated changes in prs

@NiceAesth
Copy link
Owner

NiceAesth commented Aug 24, 2023 •
edited
Loading

Could be worth re-cross checking types with osu-web. https://github.com/ppy/osu-web/tree/master/app/Models

Either way I appreciate that you went out of your time to check these issues.

@NiceAesth NiceAesth changed the title Changed: event.py Fixed typo errors in Event(BaseModel) achievement: … fix: typos in models Aug 25, 2023
@NiceAesth
Copy link
Owner

I went ahead and fixed the issues myself. Read through the changes I have made in case you wish to see the direction to take PRs in the future.

(also, as a future note I recommend reading https://towardsdatascience.com/why-good-codes-dont-need-comments-92f58de19ad2)

@NiceAesth NiceAesth merged commit 41d49bd into NiceAesth:master Aug 25, 2023
2 checks passed
@EnricoBaivo
Copy link
Contributor Author

Thanks alot for the informations, indeed it was my first pull request ever. So im looking forward to learn the correct patterns and rules for prs.

NiceAesth added a commit that referenced this pull request Aug 25, 2023
@EnricoBaivo @pre-commit-ci @NiceAesth
fix: typos in models (#164)
3749055 
* Changed: event.py Fixed typo errors in Event(BaseModel) achievement: Optional[Achievement] = None # typo achievment  and in common.py Achievement(BaseModel) description: str # typo desciption

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fixed: Achievement class parameter mode is a optional Field, possible can be null. Example is the achievement: Autocreation that has a gamemode of null

* chore: remove unrelated changes in .gitignore

* chore: revert unrelated change

* Update .gitignore

* chore: fix pr

* chore: fix pr

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: NiceAesth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NiceAesth NiceAesth NiceAesth approved these changes

Assignees
No one assigned
Labels
size/XS
Projects
aiosu
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@EnricoBaivo @NiceAesth