Skip to content

Conversation

@regisb
Copy link
Contributor

@regisb regisb commented Jan 2, 2023

Description

Just a small fix to the CONTRIBUTING docs. Previous link was a 404.

Testing instructions

Compare the following links:

https://discuss.openedx.org/c/devops
https://discuss.openedx.org/c/operators

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 2, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @regisb! 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 as you can:

  • 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.

@regisb
Copy link
Contributor Author

regisb commented Jan 3, 2023

CI is broken because we are no longer in 2022:

FAILED common/djangoapps/student/tests/test_models.py::TestUserPostSaveCallback::test_is_marketable_set_to_false_for_user_created_via_management_command - AssertionError: assert (1, {'address...': None, ...}) == (1, {'address...': None, ...})
  At index 1 diff: {'email': 'some.user@example.com', 'username': 'some_user', 'name': 'Student Person', 'age': -1, 'yearOfBirth': 2023, 'education': None, 'address': None, 'gender': 'Male', 'country': '', 'is_marketable': False} != {'email': 'some.user@example.com', 'username': 'some_user', 'name': 'Student Person', 'age': -1, 'yearOfBirth': 2022, 'education': None, 'address': None, 'gender': 'Male', 'country': '', 'is_marketable': False}
  Full diff:
    (
     1,
     {'address': None,
      'age': -1,
      'country': '',
      'education': None,
      'email': 'some.user@example.com',
      'gender': 'Male',
      'is_marketable': False,
      'name': 'Student Person',
      'username': 'some_user',
  -   'yearOfBirth': 2022},
  ?                     ^
  +   'yearOfBirth': 2023},
  ?                     ^
    )

@zainab-amir as the original author of this test, could you please provide a fix? You could for instance replace 2022 by datetime.now(UTC).year, as you did in your other test.

@zainab-amir
Copy link
Contributor

@regisb it has been fixed by another contributor. Apologies for the delay from my side.
#31488

@mphilbrick211
Copy link

Hi @regisb - just checking on this to see if there's an update?

@mphilbrick211
Copy link

Friendly ping on this @regisb :)

@mphilbrick211
Copy link

@zainab-amir @regisb - hi there! Is this pull request still being pursued?

@zainab-amir
Copy link
Contributor

@mphilbrick211 I am not sure. The issue I was originally pinged on has been resolved and this PR probably just needs a rebase by @regisb.

@regisb
Copy link
Contributor Author

regisb commented Mar 29, 2023

My apologies @mphilbrick211, I had misconfigured notifications for the edx-platform repository 😓 I have now rebased my PR and this is ready for another review.

@mphilbrick211
Copy link

My apologies @mphilbrick211, I had misconfigured notifications for the edx-platform repository 😓 I have now rebased my PR and this is ready for another review.

Thanks, @regisb! Looks like a few tests are still failing. Would you mind looking into it?

@regisb regisb requested review from a team April 5, 2023 05:25
@jristau1984
Copy link
Contributor

Is this supposed to be a change to 1300 files? If so, a T&L review is going to take a long time to complete. If not, can this please be re-submitted so that only the "small fix to the CONTRIBUTING docs" is present in the PR? Thanks!

@regisb
Copy link
Contributor Author

regisb commented Apr 6, 2023

Is this supposed to be a change to 1300 files?

No 😅 Apparently, that "update with rebase" button on the GitHub PR page is not working as intended. I have now properly rebased the PR.

Copy link
Contributor

@jristau1984 jristau1984 left a comment

Choose a reason for hiding this comment

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

Looks great now!

@jristau1984 jristau1984 merged commit b94f196 into openedx:master Apr 6, 2023
@openedx-webhooks
Copy link

@regisb 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@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.

@regisb regisb deleted the patch-1 branch April 6, 2023 13:35
@edx-pipeline-bot
Copy link
Contributor

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

1 similar comment
@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

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants