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.

Sign up for GitHub

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

E0d/namespace adr #36

Merged
merged 15 commits into from
Jan 8, 2024
+96 −0
Merged

E0d/namespace adr #36

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ae7c93c
feat: first draft of namespacing proposal
Dec 7, 2022
0db61a5
fix: spelling/ignore
Dec 7, 2022
abd6c5c
fix: spelling/grammar
Dec 8, 2022
22d170c
fix: Update docs/decisions/0001-namespacing-branches.rst
e0d Dec 8, 2022
24069a2
fix: heading standard
Dec 8, 2022
39e16ea
fix: responding the CR comments
Dec 8, 2022
4d97365
fix: improving readability
Dec 8, 2022
b1441ee
fix: clarify re personal branches.
Dec 8, 2022
044baf4
Merge remote-tracking branch 'upstream/master' into e0d/namespace-adr
kdmccormick Oct 10, 2023
71f1066
docs: fix typo in branch naming ADR
kdmccormick Jan 3, 2024
afd96ae
docs: fix typo in branch naming ADR
kdmccormick Jan 3, 2024
97076a1
docs: fix typo in branch naming ADR
kdmccormick Jan 3, 2024
0135fd5
docs: address PR comments
kdmccormick Jan 8, 2024
63f6cbc
docs: status
kdmccormick Jan 8, 2024
55e1ba7
docs: typo, word choice
kdmccormick Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

#ignore .idea directory
.idea
*~
71 changes: 71 additions & 0 deletions docs/decisions/0001-namespacing-branches.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
Namespacing Branches and Tags
-----------------------------
e0d marked this conversation as resolved.
Show resolved Hide resolved

Status
======

Draft
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved

Context
=======

This ADR was written in response to this `Discourse thread`_.

Currently, 2U continuous delevery tooling creates branches and tags
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
in the repository "global namespace." By that I mean that branches
and tags have the form ``release`` or ``release-candidate-3729``.
This is confusing because those branches and tags appear to be global,
officially sanctioned by the project, and authoritative.

A result of this is that the official named releases of the platform
require a namespace specifier, for example,
``open-release/olive.master``.

This is confusing to most members of the community and reflects the
prior stewardship arrangements for the project.

It is not typical of open-source projects to allow participating
organizations to push organization specific branches and tags to the
canonical upstream. However, we recommend not prohibiting this at this
time. There are two reasons for that:

1. This would require more substanial changes to partipant deployment
e0d marked this conversation as resolved.
Show resolved Hide resolved
systems that would delay getting value from this initial change.
2. The 2U branches and tags represent code that is battle-tested in
production at scale and are, thus, valuable sign posts for
community members.

.. _Discourse thread: https://discuss.openedx.org/t/should-we-rename-the-release-branches/8827/7


Decisions
=========

1. We will not forbid project participants with write access to
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
repositories from pushing branches or tags to the canonical
upstream hosted at the openedx GitHub organization
2. We will require that project participants namespace their branches
and tags to clarify their origin and purpose
3. The global namespace will be reserved for global project purposes
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved

Going forward branches and tags that originate from any specific
Copy link
Contributor

Choose a reason for hiding this comment

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

@e0d: If this a global decision, it should really be an OEP and follow the OEP proposal process. In this case, this would probably just serve as a useful inform. At one-time, we even had a template (or note) about an OEP that could just refer to an ADR.

Copy link
Member

Choose a reason for hiding this comment

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

I feel that this ADR is sufficient for gathering input, and sharing it out via Slack will be a sufficient inform. Once this lands, I'll open a ticket to add these rules to the Core Contributor course if they're not already there. Can you folks at 2U add these rules to your own dev onboarding docs?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm realizing that I'm not entirely clear on the rules, and thus what belongs in our onboarding docs. I'm confused because I don't know how complete the examples are.

Here are some questions/thoughts:

  • It feels like the consequence including "...guidance that personal branches..." should be part of the decision.
  • It would be helpful to get clarification about what is and what is not included in "...branches and tags that originate from any specific organization...".
    • According to the consequences, it sounds like personal branches are not included, even as employees of some organization.
    • Also, is release the only in-use and illegal branch and tag name? Is this just an example, or is this the problem? Would it be valid to reword to "... release branches and tags that originate..."? If not, are there other examples or clarifying description to add regarding what this is referring to?

Separate but related, it would be interesting to see a spreadsheet of repos, tags, and branches that are at issue here. I'm not clear what the actual impact is to 2U, and whether this impacts many repos, branches and tags, or if it just affected edx-platform, and if this has been cleaned up already? This is related to the doc request, because I'm not clear about what messaging, if any, would be appropriate for most engineers.

Copy link
Member

Choose a reason for hiding this comment

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

I'm realizing that I'm not entirely clear on the rules, and thus what belongs in our onboarding docs. I'm confused because I don't know how complete the examples are.

I think the ADR is clear. Feel free to open a PR against this one if you think it could be organized or worded better.

According to the consequences, it sounds like personal branches are not included, even as employees of some organization.

Correct.

Also, is release the only in-use and illegal branch and tag name? Is this just an example, or is this the problem? Would it be valid to reword to "... release branches and tags that originate..."? If not, are there other examples or clarifying description to add regarding what this is referring to?

Philosophically, we would like 2U and CCs to act in good faith and namespace their branches appropriately, always. In practice, we don't plan to be jerks about it; would give a heads up and sufficient time to adapt before deleting anything. There are dozens of branches and tags which conflict with this ADR, but release and release/* are the only ones which pose a concrete problem right now, so they are the only ones we are hoping to delete as early as March.

Separate but related, it would be interesting to see a spreadsheet of repos, tags, and branches that are at issue here. I'm not clear what the actual impact is to 2U, and whether this impacts many repos, branches and tags, or if it just affected edx-platform, and if this has been cleaned up already?

See #36 (comment) for branches. I'll re-ping later once I'm able to update those JSON docs to also include tags.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding the documentation ask:

Generally, we just need to make sure that people with upstream write access understand this guidance, since I don't expect new devs to be looking through .github ADRs. Currently, there are only two groups with upstream write access: CCs, and 2U. I can take care of making sure that CCs see this guidance, probably via the CC onboarding course. What we need from 2U is to make sure their employees understand it as well, since they won't all be taking CC onboarding. However you think it'd be best to communicate that, and whether it's public or private, is fine by me.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kdmccormick: Thank you for adding the clarity I needed. I was having trouble determining whether the ADR was focused on the concrete problem or the philosophical problem, especially as it related to any deadline.

Regarding a variety of points from the above thread:

  1. I still have no idea why this isn't an OEP. There are benefits to that process, and I still don't get why this would be an exception, or treated any differently from conventional commits (as an example).
  2. I think it would be more clear and helpful to others to call out the reserved keywords like "release" that will be targeted for deletion, from all other branches which are philosophically a problem, but aren't causing any concrete issues at this time. Or maybe the ADR could note different types of treatment for branches created before or after a certain date? I'm just looking for simple ways to codify in the text the following sentiment: "In practice, we don't plan to be jerks about it.", especially given the number of non-namespaced branches/tags.
  3. New topic (related to documentation): [food for thought] I think one of the most helpful things for any OEP like this is automation, like that used for conventional commits. I don't know if there are ways to prevent new branches/tags that don't match the convention? If not, maybe a job could detect new issues and simply notify users? Maybe just a comment on PRs with non-namespaced branch names would be enough to help spread the word?

Copy link
Member

Choose a reason for hiding this comment

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

  1. An OEP is a greater time investment than an ADR. It takes longer to write and review. It invites all sorts of comments around grammar and formatting and "What about $RELATED_PROBLEM?". I think that extra time investment into consensus-building is worthwhile it when the decision at hand is especially important, impactful, controversial, and/or complex. In this case, though, we are talking about something that is, in the grand scheme of things, unimportant--the connection between branch names and learner outcomes is very indirect. It's also not controversial--this has been an unwritten best-practice as long as I can remember working on this project. It's not complex either--I don't think that protracted discussion with a wide array of community members will yield a substantially better decision. So, it's not only that I don't want to make this an OEP; I believe it would actually be an irresponsible use of my employer's time, which the community is counting on to be spent delivering deeply-needed improvements to the platform itself.

2+3) Your comments have made me realize that this ADR is actually being too specific. I'm going to remove the mention of the March 11 deadline. This ADR is simply deciding that branches should be namespace by GH username or organization. Once this merges, we will make a judgement call on which branches will be removed when, and communicate that out with ample time for 2U to adjust.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. You don't have to change gears, but some thoughts regarding OEP:

  1. I think OEPs are the right way to communicate cross-repo decisions.
  2. In my opinion, there is nothing fixed about the OEP process that forces it to take a greater time investment. You could simply note in the PR or announcement that you are mostly documenting a past decision, and that you are not leaving it open for input, etc. Anyway can always fix typos or adjust in the future.
    3. Related, we wanted to make OEPs quick to write, so there is a template that allows you to refer to an ADR. So, even once this lands, someone could loop back with an OEP that points to this decision, and doesn't open the door for additional overhead, simply to allow it do be documented in the expected location.

Note that I've short-circuited DEPR process details when I knew I was going to DEPR something for my own reasons, but I still wanted to use it for the inform.

In any case, all food for thought. Thanks and good luck.

organization partipatinging in the Open edX project must by namespaced
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
as follows:

``acme/release``

or

``acme/release-candidate-3729``


Consequences
===========

1. Continuous delivery systems and other automation should be updated
across the ecosystem to use the proper naming convention for
branches and tags
2. Existing branches and tags should be updated to use the new naming
protocol
3. A reasonable grace period should be established to schedule and
complete the required work