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

Checkpoint 1 Review #31

Closed
wants to merge 182 commits into from
Closed

Checkpoint 1 Review #31

wants to merge 182 commits into from

Conversation

sundayguru
Copy link
Owner

No description provided.

sunday Nwuguru added 30 commits April 10, 2016 21:10
…e, I also added listallocation method to allocationapp class
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d1f16c7 on master into * on review*.

One stop solution for room allocation management

### Badges

Choose a reason for hiding this comment

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

Move the badges to below the README title.


if int(row['allocated']) > 0:
db.table_name = 'person'
people = db.execute('SELECT * FROM person WHERE assigned_room LIKE "%'+ room.name +'%"')

Choose a reason for hiding this comment

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

Code Quality: use of .format() is a cleaner way of concatenating strings that using +.

@andela-anandaa
Copy link

andela-anandaa commented May 12, 2016

README

  • Your README is okay, except for a few nitpicks (in-line) above.

Github Commits

  • Your commit messages are not consistent, please refer to the guidelines here

Comments

  • Fairly commented code, but some methods are missing docString comments.

Code Quality

  • See in-line comments above.

Functionality

  • Typo in the message: "Successful" -> "Successfully"
  • Added 2 more functionalities: allocate and remove person; displaying list of unallocated rooms

Testing

  • Good attempt at writing extensive tests
  • Good: integrating with TravisCI and Coveralls

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 867f918 on master into * on review*.

@sundayguru sundayguru closed this May 12, 2016
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.

None yet

3 participants