West Midlands | 25-ITP-May | Emin Akturk | Sprint 3 | Module Data Groups #764
Conversation
eminakturk
added
the
Needs Review
wilsonadenuga
added
Review in progress
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
1 similar comment
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
wilsonadenuga
left a comment
wilsonadenuga
left a comment
There was a problem hiding this comment.
Good job here @eminakturk just some fixes here and there.
There was a problem hiding this comment.
Implementation looks good. a couple of things to fix
- what would happen if a negative number is entered
- If I don't type in any value and click on set alarm, the app behaves in an unexpected way. Can you fix this?
There was a problem hiding this comment.
Good implementation; it would be nice to add some CSS to make the app look nicer.
There was a problem hiding this comment.
Good job; there are some minor discrepancies in the final output vs. the expected output
Yours,
Expected
Notice the position of the author and title
There was a problem hiding this comment.
Good job here, just a few things to fix.
- Your design does not match the expected design
Your Interfaces
(Wrong layout)
(Overflowing Image)
You can check the expected outcome here
https://github.com/eminakturk/Module-Data-Groups/blob/d780bd2eff5b0a8de65c16fdb8144299775fc6b3/Sprint-3/slideshow/example-screenshots/example-level2.png
HINT: you can give the image a defined width and height to prevent it from overflowing
wilsonadenuga
added
Reviewed
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
1 similar comment
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
eminakturk
added
the
Needs Review
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
wilsonadenuga
added
Review in progress
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
2 similar comments
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
|
Changes look good! |
wilsonadenuga
added
Complete
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
1 similar comment
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
eminakturk
changed the title
|
Your PR couldn't be matched to an assignment in this module. Please check its title is in the correct format, and that you only have one PR per assignment. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
eminakturk
changed the title
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
eminakturk
changed the title
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
eminakturk
changed the title
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
eminakturk
changed the title
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Wrong number of parts separated by |s |
eminakturk
changed the title
|
Your PR couldn't be matched to an assignment in this module. Please check its title is in the correct format, and that you only have one PR per assignment. If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
3 participants
Learners, PR Template
Self checklist
Changelist
I am aware of the indented coursework where you could see sprint 1 and sprint 2 fixes as well as sprint 3, I'm aware of the situation I will be fixing it with my next module PR.
Questions
Ask any questions you have for your reviewer.