-
Notifications
You must be signed in to change notification settings - Fork 2
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
CTP-4088 Marker view #12
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone off topic and added comments outside the scope of this change.
Please implement, defer, or ignore these suggestions as felt appropriate.
Reduce duplicate code and reuse the same template for feedback and marking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Hi both, i want to get this right so lets hang and go through what should be: static / called by self:: before i make any further commits. I'm not a php expert, and value both your opinions so i can get better at this and know in future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
If i have a user role where the architype is editingteacher, show me summative assessments that i need to add feedback for.