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

Added ID into CustomColumn model so API documentation is accurate. #1224

Closed
wants to merge 3 commits into from

Conversation

justin0022
Copy link
Contributor

Left description for teacher_notes blank, as I don't know exactly what it does.

@CLAassistant
Copy link

CLAassistant commented Feb 2, 2018

CLA assistant check
All committers have signed the CLA.

@spencerolson
Copy link
Contributor

Hey @justin0022 , thanks for your contribution! The "teacher_notes" column is the column that will be shown/hidden when a teacher selects to show/hide notes in the Gradebook (other custom columns will not be affected by this action). Maybe you can change the description to something like:

"When true, this column's visibility will be toggled in the Gradebook when a user selects to show or hide notes"

Once you've updated the description, I'll make sure this gets in front of the appropriate team for code review.

Thanks,
Spencer

@justin0022
Copy link
Contributor Author

Hi @spencerolson, thanks!
I've updated the description now!

@spencerolson
Copy link
Contributor

Hi @justin0022 , thanks for making that change :D I'll get this in front of devs for code review and keep you updated.

@justin0022
Copy link
Contributor Author

Thanks @spencerolson!

@@ -26,6 +26,16 @@
# "id": "CustomColumn",
# "description": "",
# "properties": {
# "id": {
# "description": "The ID of the custom gradebook column",
# "example": "2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @justin0022 , can you push up a new commit with this example value ("2") unquoted? Since it's an integer, we want to drop the quotes here in the example.

@spencerolson
Copy link
Contributor

Hi @justin0022 , I left a comment on your commit. Can we unquote the example value ("2")? Once that's done, I think this will be ready to merge :D

Thanks,
Spencer

@justin0022
Copy link
Contributor Author

Good point. I've made the change now.

@spencerolson
Copy link
Contributor

Wow! You're speedy. Thanks for doing that so quickly. Running it through tests again and then I think we'll be good-to-go.

instructure-gerrit pushed a commit that referenced this pull request Feb 5, 2018
closes GH-1224
closes GRADE-824

Change-Id: Icf4ea1114d3e6661e775b424b2fb4cb186307f10
Reviewed-on: https://gerrit.instructure.com/139763
Tested-by: Jenkins
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Spencer Olson <solson@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
@spencerolson
Copy link
Contributor

Hi @justin0022 , your commit has been merged to the master branch and can be found here: bccdadb .Thanks again for your contribution!

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