Skip to content

Conversation

@arslanashraf7
Copy link
Contributor

@arslanashraf7 arslanashraf7 commented Mar 10, 2021

Related ticket:

mitodl#244

Description

Adds annotations for DISABLE_COURSE_CREATION feature flag.

Other information

There are a few points of explanation for this PR,

  • Since this flag was never added into cms/envs/common.py and it was only used with a get call with default value as False, so for the sake of consistency I've added it to cms/envs/common.py.
  • The value I've used for the toggle_creation_date is based on the earliest usage of this flag I could find in the codebase.
  • The value(None) used for toggle_tickets is because there was no specific ticket that would add this flag specifically, there are multiple PR that just have the usage of this.

@openedx-webhooks
Copy link

Thanks for the pull request, @arslanashraf7! I've created OSPR-5661 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 10, 2021
@natabene natabene changed the title Added annotations for DISABLE_COURSE_CREATION feature flag [TSD] Added annotations for DISABLE_COURSE_CREATION feature flag Mar 10, 2021
@natabene
Copy link
Contributor

@arslanashraf7 Thank you for your contribution. @eLRuLL @BbrSofiane Could you please review?

@arslanashraf7
Copy link
Contributor Author

@pdpinch FYI.

Copy link
Contributor

@BbrSofiane BbrSofiane left a comment

Choose a reason for hiding this comment

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

@arslanashraf7 Can you be a bit more descriptive in the toggle_description? Does it remove the 'New Course' button on the studio side?

@arslanashraf7 arslanashraf7 force-pushed the arslan/244-disable-course-creation-annotation branch from 4861ee8 to b32d468 Compare March 11, 2021 13:02
@arslanashraf7
Copy link
Contributor Author

@BbrSofiane, You're correct it hides the "New Course" button the I missed to add this, I've updated the annotations & PR description.

Please have a look & let me know what you think of it now.

Copy link
Contributor

@BbrSofiane BbrSofiane left a comment

Choose a reason for hiding this comment

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

Nice @arslanashraf7! @edx/doc-a-thon-reviewers this is ready to be merged,

@natabene
Copy link
Contributor

@felipemontoya @bradenmacdonald Please merge when you have a chance.

@pdpinch
Copy link
Contributor

pdpinch commented Mar 13, 2021

@arslanashraf7 can you update the commit message to follow the Conventional Commits guidelines? I think all you need is to put "docs: " at the beginning of the commit message.

@arslanashraf7 arslanashraf7 force-pushed the arslan/244-disable-course-creation-annotation branch from b32d468 to 9e41379 Compare March 14, 2021 12:38
@arslanashraf7
Copy link
Contributor Author

@arslanashraf7 can you update the commit message to follow the Conventional Commits guidelines? I think all you need is to put "docs: " at the beginning of the commit message.

@pdpinch, Thanks for pointing it out. I've updated the commit message as per the conventional commit guidelines.

@edx-status-bot
Copy link

Your PR has finished running tests. There were no failures.

@pdpinch pdpinch merged commit 8163d8c into openedx:master Mar 15, 2021
@openedx-webhooks
Copy link

@arslanashraf7 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

@pdpinch pdpinch deleted the arslan/244-disable-course-creation-annotation branch March 15, 2021 12:59
@arslanashraf7
Copy link
Contributor Author

@pdpinch, thanks for taking out the time to review and merge this PR.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@natabene
Copy link
Contributor

@BbrSofiane @pdpinch Thank you both!

@natabene
Copy link
Contributor

@arslanashraf7 Are you in Discourse? If yes, please let me know your username and I will grant you a badge for your participation in Doc-a-thon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants