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

feat!: replace old demo course with the new one #47

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Jan 10, 2024

This subsumes the openedx-demo-course-new into this repo, and removes the entirety of the old demo course. We will archive openedx-demo-course-new after merging this PR.

Notes for reviewers:

  • I made this PR by rm'ing the current repository and replacing it with the contents of https://github.com/openedx/openedx-demo-course-new, plus just a couple tweaks:
    • I changed the GHA CI branches from main back to master.
    • I put openedx.yaml back so this this repo continues to be released.
    • I fixed the repo name in catalog-info.yaml
  • The insane -400k diff is probably because the jsmol library had been entirely copied into this repo previously.
  • Rather than trying to read the diff, I would just browse the branch a bit as a sanity check.

Operator Impact

On its own, this PR will break the tutor local do importdemocourse command in Tutor Nightly. Merging this update to Tutor Nightly will fix the command again. With that PR merged:

  • For Quince (and earlier) users, the default remains the old course. Specifically:
    • Get the old course with tutor local do importdemocourse
    • Get the new course with: tutor local do importdemocourse demo-course/course --version master && tutor local do importdemolibraries --version master
  • For master and Redwood+ users, the default becomes the new course. Specifically:
    • Get the old course with: tutor local do importdemocourse --version open-release/quince.1
    • Get the new course and library with: tutor local do importdemocourse && tutor local do importdemolibraries

@kdmccormick kdmccormick force-pushed the kdmccormick/new-demo-course branch from a1d752b to 65c9cea Compare January 10, 2024 17:19
The old demo course can still be accessed by checking out
named release tags of this repo.
@kdmccormick
Copy link
Member Author

@sarina One last PR for you! With this one merged and the linked Tutor PR merged, we'll be fully prepped to switch over to the new demo course for Redwood.

@kdmccormick kdmccormick requested a review from sarina January 10, 2024 17:58

on:
pull_request:
branches: ["master"]
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be "main"?

Copy link
Contributor

Choose a reason for hiding this comment

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

nevermind I misunderstood this PR, read more carefully now. GTG

@sarina
Copy link
Contributor

sarina commented Jan 10, 2024

This looks good, before I give thumbs-up I want to do a quick test of the tar.gz into my personal instance, which I'll do now

Edit: I'm getting errors; I'll work with John to resolve them this week.

@sarina
Copy link
Contributor

sarina commented Jan 18, 2024

This should be updated with the changes made in openedx-unsupported/openedx-demo-course-new#4 - once that's done we can go ahead and merge.

kdmccormick and others added 2 commits January 18, 2024 13:27
When temporary operating-system-specific files like .DS_Store
(a macOS Finder artifact) are packed into the dist/ tars, it
creates version control noise and can make the source-matches-tars
check fail unnecessarily.

This updates `make dist` to first `git clean` away any git-ignored
files. It also changes `make unpack` to work in the case where the
source directories are missing, which is useful when you want to
unpack the tars from scratch.

Finally, this commit re-runs `make dist`, so that the dist/
tars no longer have unnecessary OS-specific files in them.
@kdmccormick
Copy link
Member Author

@sarina I pushed two commits, ready for your review:

  1. Copy in @jswope00 's recent changes: ab4a57a
  2. Remove some macOS hidden-file-cruft from the OLX tars, and update the Makefile so that cruft doesn't get added to the tars next time: 59e6320

I just need to address Régis's comments over on the Tutor PR, and then I think we're all set!

Copy link
Contributor

@sarina sarina left a comment

Choose a reason for hiding this comment

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

Thanks Kyle, merge away when ready!

@sarina
Copy link
Contributor

sarina commented Jan 29, 2024

Hey @kdmccormick - we merged in one more change to https://github.com/openedx/openedx-demo-course-new - it'd be good to get this updated & merged, so we can close down the openedx-demo-course-new repo and churn on this PR. Let me know the best way to proceed - should we merge this and port the changes from the last PR in a separate PR?

@kdmccormick
Copy link
Member Author

@sarina I was holding off on merging this because it'll break importdemocourse in Tutor Nightly unless the Tutor PR is merged first. The drift between this and openedx-demo-course-new isn't good, though, so I'm going to merge this now and the Tutor fix will be a fast follow.

@kdmccormick kdmccormick merged commit 4104aa8 into master Feb 6, 2024
3 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/new-demo-course branch February 6, 2024 17:05
timmc-edx added a commit to openedx-unsupported/devstack that referenced this pull request Feb 7, 2024
timmc-edx added a commit to openedx-unsupported/devstack that referenced this pull request Feb 7, 2024
In openedx/openedx-demo-course#47 the demo
course was replaced. This includes a new course key (new name and org)
and the course content being moved a few directories deeper.

Changes:

- `sed -i 's/course-v1:edX+DemoX+Demo_Course/course-v1:Axim+DemoX+demo_course/g'`
  on all files turned up by `grep -rl Demo_Course`
- Updated a few other course info locations turned up with
  `grep -nrI DemoX | grep -x demo_course` (all in programs/discovery.py)
- Updated git clone URL and checkout location in provision-lms.sh to
  use new repo name (not strictly needed).
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.

2 participants