Skip to content

Pei Lin He - completed onboarding (backend)#44

Open
cryolins wants to merge 6 commits intoUWOrbital:mainfrom
cryolins:main
Open

Pei Lin He - completed onboarding (backend)#44
cryolins wants to merge 6 commits intoUWOrbital:mainfrom
cryolins:main

Conversation

@cryolins
Copy link

@cryolins cryolins commented Sep 19, 2025

Purpose

Completed the GS on-boarding task. Include a screenshot of the front-end of the application.

  • I have added my name to the onboarding title (required)
  • I have added a screenshot of the logs printed by the logger middleware (required)
  • I am interested to work on the frontend (optional)

New Changes

  • Explain new changes

Testing

  • Explain tests that you ran to verify code functionality.
image

Copy link

@Adityya-K Adityya-K left a comment

Choose a reason for hiding this comment

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

Great first push! Just two things to fix and you will be onboarded!

db.delete(item)
db.commit()

items = db.exec(select(Command)).all()

Choose a reason for hiding this comment

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

This seems to be repeated functionality! You can use the get_commands() function here instead!

return self
if(self.params is None or self.format is None
or self.params.count(",") != self.format.count(",")):
raise ValueError()

Choose a reason for hiding this comment

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

Can you make this value error have a description (i.e. ValueError("Description here"))

Copy link

@Adityya-K Adityya-K left a comment

Choose a reason for hiding this comment

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

LGTM!

@Adityya-K Adityya-K closed this Sep 20, 2025
@Adityya-K Adityya-K reopened this Sep 20, 2025
@cryolins
Copy link
Author

okay, and here's a frontend pic just in case it's needed
image

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