Skip to content

chore: pin GitHub Actions workflows to full commit SHAs#246

Open
irfanuddinahmad wants to merge 5 commits into
openedx:masterfrom
irfanuddinahmad:pin-actions-to-sha
Open

chore: pin GitHub Actions workflows to full commit SHAs#246
irfanuddinahmad wants to merge 5 commits into
openedx:masterfrom
irfanuddinahmad:pin-actions-to-sha

Conversation

@irfanuddinahmad

@irfanuddinahmad irfanuddinahmad commented May 19, 2026

Copy link
Copy Markdown

Pins all uses: action refs to their full commit SHA with the version tag preserved as a comment. Part of org-wide SHA-pinning migration: openedx/.github#165

@irfanuddinahmad irfanuddinahmad mentioned this pull request May 19, 2026
Closed
@farhan farhan mentioned this pull request May 25, 2026
Closed
@feanil

feanil commented May 30, 2026

Copy link
Copy Markdown

@irfanuddinahmad looks like the PR is failing tests, probably unrelated, can you fix the issue anyway in a separate PR and then we can rebase this and land it.

Irfan Ahmad and others added 2 commits June 2, 2026 16:56
@claude
fix: add setuptools to requirements for Python 3.12 compatibility
4dac5d3 
enmerkar 0.7.1 calls `from pkg_resources import get_distribution` in
its __init__.py at runtime. pkg_resources ships with setuptools, which
is no longer bundled with Python 3.12. Adding setuptools as an explicit
requirement ensures it is available in the tox virtualenv.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
fix: add conftest.py shim for pkg_resources when setuptools unavailable
d1241da 
enmerkar 0.7.1 imports pkg_resources at module load time but does not
declare setuptools as a dependency. In some virtualenv configurations,
pkg_resources is not importable even when setuptools is installed.
This conftest.py creates a minimal importlib.metadata-backed shim so
that pytest collection succeeds regardless of the virtualenv setup.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feanil
feanil reviewed Jun 3, 2026
View reviewed changes

@feanil feanil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, worth checking is where this library is being used, do we still need this fork?

Comment thread conftest.py Outdated
@irfanuddinahmad

irfanuddinahmad commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Who uses it: Only openedx/openedx-platform is a direct dependent (pinned at enmerkar-underscore==2.4.0 in requirements/edx/kernel.in and related files). edx-enterprise pins it only as a platform constraint, not a direct dependency. No other repos in the openedx or edx orgs depend on it.

Do we still need the fork? Yes, for now. The upstream (EnTeQuAk/django-babel-underscore) last pushed in January 2021 and has never shipped Django 4.x/5.x or Python 3.11/3.12 support. This fork exists precisely to maintain that compatibility — it added Django 4.2/5.2 support and Python 3.11/3.12 support (the latter required vendoring markey since that upstream is also defunct and a Python 3.11 compat PR there was never merged).

As long as openedx-platform uses Underscore.js templates requiring i18n extraction, this fork is needed — there's no drop-in alternative on PyPI with the same underscore Babel extractor entry point and modern Django/Python support. The question of whether it's still needed would ultimately hinge on whether the platform has migrated (or is planning to migrate) away from those Underscore.js templates.

@openedx-webhooks

openedx-webhooks commented Jun 10, 2026

Copy link
Copy Markdown

Thanks for the pull request, @irfanuddinahmad!

This repository is currently unmaintained.

🔘 Find a technical reviewer To get help with finding a technical reviewer, reach out to the community contributions project manager for this PR:
  1. On the right-hand side of the PR, find the Contributions project, click the caret in the top right corner to expand it, and check the "Primary PM" field for the name of your project manager.
  2. Find their GitHub handle here.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Jun 10, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Jun 10, 2026
@claude
refactor: vendor enmerkar/extract.py, drop enmerkar dep and pkg_resou…
ca2fe09 
…rces shim

enmerkar 0.7.1 does `from pkg_resources import get_distribution` in its
__init__.py, which fails in Python 3.12+ environments where setuptools is
not installed. We only use `extract_django` from enmerkar/extract.py, and
that module has no pkg_resources dependency.

Vendor enmerkar/extract.py directly (as already done for markey) so we
use importlib.metadata-compatible code throughout, then drop the enmerkar
package dependency, the setuptools pin, and the conftest.py shim.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mphilbrick211 mphilbrick211 moved this from Needs Triage to In Eng Review in Contributions Jun 10, 2026
@irfanuddinahmad
irfanuddinahmad requested a review from feanil June 10, 2026 14:09
@farhan
farhan self-requested a review June 11, 2026 09:07
@farhan

farhan commented Jun 11, 2026

Copy link
Copy Markdown

@feanil @kdmccormick
I think repo owner review is must in this PR or can you guys review it?

feanil
feanil requested changes Jun 24, 2026
View reviewed changes

@feanil feanil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Generally makes sense, just one bit of organizing to be able to not expose ourselves to legal troubles.

Comment thread src/enmerkar_underscore/vendor/enmerkar/enmerkar_extract.py

@feanil feanil Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This makes sense, we eventually want to fully drop the django underscore tooling but this is a good intermediate step to reduce un-necessary dependencies by taking on some more probably fairly minimal maintenance burden in the near term.

When you're vendoring in code from other libraries, it's really important to read their License. In this case, we should copy the COPYING file next to this file in the vendor directory.

I think it's worth it to rename COPYING to LICENSE here for clarity and to put the vendored enmerkar code into a folder named for the library it's coming from. It's unlikely that we'll have multiple vendored items in this repo but it's good practice to have a separate vendor folder for each upstream item you're vendoring. It has the side benefit that we can keep the name of this file the same as the upstream file it's copying which also makes provenance easier.

@claude
chore: move vendored enmerkar into its own subfolder with LICENSE
2dc2e2c 
Add upstream BSD-3-Clause LICENSE (from Zegocover/enmerkar COPYING) and
move enmerkar_extract.py into vendor/enmerkar/ per code review feedback.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@feanil feanil feanil requested changes
@farhan farhan Awaiting requested review from farhan

Assignees

No one assigned

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Contributions
Status: In Eng Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@irfanuddinahmad @feanil @openedx-webhooks @farhan @mphilbrick211