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

Refactoring #14

Merged
merged 6 commits into from
Jan 21, 2024
Merged

Refactoring #14

merged 6 commits into from
Jan 21, 2024

Conversation

GabrielBarberini
Copy link
Collaborator

  • Improve code style and readability
  • Group API routes by tag as different modules
  • Fix pylint errors

@GabrielBarberini
changes compose naming
83b0e4e 
refactors repositories

standardizes controller exception handling

refactors application routes

fixes repositories connection close timing

updates postman collection

updates README

fixes pylint errors

fixes pylint errors

apply black style to modules

Update pylint.yml

Update pylint.yml

Update pylint.yml

disable warnings
@GabrielBarberini
Copy link
Collaborator Author

JFYI: All integration tests passed for this.

Gui-FernandesBR
Gui-FernandesBR reviewed Jan 11, 2024
View reviewed changes
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Overall it's a very good work!

When reviewing 29 files, I may not be able to catch all the details, but I tried my best, hope you can find it helpful.

lib/models/motor.py Outdated Show resolved Hide resolved
lib/controllers/environment.py Show resolved Hide resolved
lib/controllers/rocket.py Show resolved Hide resolved
lib/models/rocket.py Outdated Show resolved Hide resolved
lib/models/rocket.py Outdated Show resolved Hide resolved
lib/routes/environment.py Show resolved Hide resolved
lib/api.py Show resolved Hide resolved
lib/api.py Show resolved Hide resolved
@GabrielBarberini
Copy link
Collaborator Author

I will have to remove W0511 from pylint as well since we will persist using #TODO for now

@GabrielBarberini
Update pylint.yml
e1f8f9c 
removes W0511 (tolerates #TODO)
Gui-FernandesBR
Gui-FernandesBR approved these changes Jan 21, 2024
View reviewed changes
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Good job @GabrielBarberini !!

@GabrielBarberini GabrielBarberini merged commit 7ac18eb into master Jan 21, 2024
3 checks passed
@GabrielBarberini GabrielBarberini deleted the refactoring branch January 21, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gui-FernandesBR Gui-FernandesBR Gui-FernandesBR approved these changes

@aasitvora99 aasitvora99 Awaiting requested review from aasitvora99 aasitvora99 is a code owner

@luimot luimot Awaiting requested review from luimot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GabrielBarberini @Gui-FernandesBR