Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

build: tag this in future Open edX releases #52

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

kdmccormick
Copy link
Contributor

@kdmccormick kdmccormick commented Oct 10, 2023

build: don't tag this in the named release

@bmtcril , @cmltaWt0 mentioned that we shouldn't tag this in the release. Is that because it's installed via pip requirements?

UPDATE:

build: tag this in future Open edX releases

...because it's an optional plugin that will eventually be used by
the open source community.

@bmtcril
Copy link
Contributor

bmtcril commented Oct 10, 2023

This one is just related to the Aspects work which hasn't hit a v1 yet. It will be installed by pip requirements via the Aspects Tutor plugin when it does land, though, so maybe still doesn't need to be in the release?

@Ian2012
Copy link
Contributor

Ian2012 commented Oct 10, 2023

We should wait till this lands a stable release (formerly aspects-v1)

@kdmccormick
Copy link
Contributor Author

kdmccormick commented Oct 10, 2023

Ah, I see. We generally don't factor development status in when deciding whether to tag a repo. The decision tree for including repos in the release is:

  • Is it a pip/npm-installed by another application?
    • Yes -> don't tag it.
    • No -> Will it ever be in the Open edX release?
      • Yes -> Tag it! What state is it in?
        • Prod-ready -> enable it in the release.
        • Beta-ready -> leave it disabled, but advertise it as "experimental" in the release notes.
        • Not ready -> leave it disabled, don't mention it in the release notes.
      • No -> Don't tag it.

In this case, I think that means we would tag it but leave it disabled. If that sounds right @bmtcril , then I can make that change to openedx.yaml.

We should apply the same decision tree for openedx/event-bus-redis#53 as well.

@bmtcril
Copy link
Contributor

bmtcril commented Oct 10, 2023

Hmm, I think that since it would be pip installed in edx-platform we wouldn't tag it?

@kdmccormick
Copy link
Contributor Author

Yeah, that idea is that we don't want to "double-require" something. You can think of the tags as the top-level requirements of the Open edX release. If a package were required by edx-platform and we tagged it, then the package would end up with two (potentially divergent) versions: the version that edx-platform pins, and the named release tag.

So, if this is (or will be) a direct dependency of edx-platform, then we shouldn't tag. But if this is an optional plugin to edx-platform, then we should tag it.

@bmtcril
Copy link
Contributor

bmtcril commented Oct 10, 2023

Got it, then I think tagged -> experimental makes the most sense. Thanks!

@kdmccormick
Copy link
Contributor Author

Sounds good! Would you say the same for openedx/event-bus-redis#53 ? It's OK that openedx-events isn't tagged; it's used by several repos that are tagged.

@bmtcril
Copy link
Contributor

bmtcril commented Oct 10, 2023

Sure, makes sense

@kdmccormick kdmccormick changed the title docs: don't tag this in the named release build: tag this in future Open edX releases Oct 10, 2023
...because it's an optional plugin that will eventually be used by
the open source community.
@kdmccormick kdmccormick marked this pull request as ready for review October 10, 2023 18:55
@kdmccormick
Copy link
Contributor Author

I updated both PRs. Now, instead of removing the packages from the next release, the PRs will add this package to Redwood and beyond.

I think it's OK that we didn't tag them for Quince.

@cmltaWt0 could you review this?

@cmltaWt0
Copy link

I updated both PRs. Now, instead of removing the packages from the next release, the PRs will add this package to Redwood and beyond.

I think it's OK that we didn't tag them for Quince.

@cmltaWt0 could you review this?

Since this is IDA, it make sense to me to.
Will be tagged and marked as experimental in the next release 👍

@kdmccormick
Copy link
Contributor Author

Thanks for the review @cmltaWt0 .

Since this is IDA, it make sense to me to.

Just to clarify, this is a plugin, not an IDA.

@kdmccormick kdmccormick merged commit 656b7a8 into main Oct 10, 2023
8 of 9 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/no-release-tag branch October 10, 2023 20:00
@cmltaWt0
Copy link

Just to clarify, this is a plugin, not an IDA.

I messed up, hard day. Thanks!
Will note it in Redwood wiki.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants