Skip to content

Conversation

@syedimranhassan
Copy link
Contributor

Please consider the following when opening a pull request:

  • Link to the relevant JIRA ticket(s) and tag any relevant team(s).
  • Squash your changes down into one or more discrete commits.
    In each commit, include description that could help a developer
    several months from now.
  • If running make upgrade, run as close to the time of merging as possible
    to avoid accidentally downgrading someone else's package.
    Put the output of make upgrade in its own separate commit,
    decoupled from other code changes.
  • Aim for comprehensive test coverage, but remember that
    automated testing isn't a substitute for manual verification.
  • Carefully consider naming, code organization, dependencies when adding new code.
    Code that is amenable to refactoring and improvement benefits all platform developers,
    especially given the size and scope of edx-platform.
    Consult existing Architectural Decision Records (ADRs),
    including those concerning the app(s) you are changing and
    those concerning edx-platform as a whole.

@syedimranhassan syedimranhassan force-pushed the ihassan/OPS-3772_clean_up_rendering_code_cms branch 2 times, most recently from b7658af to 6a5a901 Compare October 11, 2019 11:20
@syedimranhassan
Copy link
Contributor Author

syedimranhassan commented Oct 11, 2019

TIME_ZONE = 'America/New_York'
DOC_STORE_CONFIG['password'] = 'password'
Feature['ENABLE_THIRD_PARTY_AUTH'] = True

These variable cause tests to fail when I added them. I have tried to modify test file as well but still failing.

@coryleeio
Copy link
Contributor

If they change the behavior lets leave them out - the intent is that we wouldn't be changing the behavior so I think that's fine. It is a bit weird that it's changing. It is probably tests encoded that are making expectations based on the TMZ. Doc store auth breaking makes some sense. Not sure about third party auth, but being off seems like a reasonable default for that anyway.

@syedimranhassan syedimranhassan force-pushed the ihassan/OPS-3772_clean_up_rendering_code_cms branch from 6a5a901 to 559e280 Compare October 17, 2019 07:28
@edx-status-bot
Copy link

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

@syedimranhassan syedimranhassan merged commit 65c7332 into master Oct 17, 2019
@syedimranhassan syedimranhassan deleted the ihassan/OPS-3772_clean_up_rendering_code_cms branch October 17, 2019 08:06
@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 on Thursday, October 17, 2019.

@edx-pipeline-bot
Copy link
Contributor

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

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.

5 participants