-
Notifications
You must be signed in to change notification settings - Fork 0
Docs/fork maintenance guide #8
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
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,263 @@ | |||
# OpenHands Fork Maintenance Guide | |||
|
|||
This guide outlines a strategy for maintaining a fork of the main OpenHands repository while incorporating custom patches and ensuring stability and security. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI. This was written using openhands but guided by some ideas I've had.
2. **Security & Intentional Updates:** Avoid automatic updates from upstream; intentionally review and integrate changes. | ||
3. **Local Patch Management:** Easily add, manage, and remove custom features or external PRs. | ||
|
||
## 1. Initial Setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't konw if this is a useful section here, but there's a bigger idea around this.
A lot of the challenge of maintaining a fork is the mechanics of pulling from upstream, resolving conflicts, and updating the code as appropriate.
I believe we can simplify this with openhands + a cron. Every monday a job can run that will grab the latest main from upstream (or perhaps the latest official release) and do the update process.
|
||
**a. Building Images:** | ||
|
||
* **Source Branch:** Images should be built by CI/CD processes triggered by commits to the `release/stable-with-patches` branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we want to build and host these images? These will be used by github actions and local running of openhands.
We currently host packages in dockerhub, ECR, and github packages (generally not images, but gems and stuff).
I like github packages for development tools, but we could double down on ECR if that keeps things simpler.
Should these images be public or private? At the moment this is a public fork. There's nothing specific to our code or architecture here, just a couple of neat features we would use that may or may not be on their way into upstream.
|
||
* **Source Branch:** Images should be built by CI/CD processes triggered by commits to the `release/stable-with-patches` branch. | ||
* **Build Process:** Use the standard OpenHands build process (`docker build`, `make`, etc.). | ||
* **Base Image Consideration:** While the image is built *from* the code in `release/stable-with-patches`, consider how the underlying base OS/dependency image specified in the Dockerfile is kept up-to-date. This might involve periodically updating the base image tag in the Dockerfile itself as part of the fork maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks like having either an implicit or explicit "base commit" for the entire fork.
I.e. our fork may be 0.33
+ some patches. When 0.34
is released, we can update it to look exactly like 0.34
+ some patches. If we do that right, we can programmatically determine the commit we're based on (since that commit will be a part of git history) and use that same tag for the base in our docker images.
I feel like this should already be a solved problem, but I am not familiar with anything that keeps this enforced and simple. Anyone ever done this before? My guess is that you just use a build arg in the dockerfile and pass in the base image for the commit we're built on.
# Example: Build triggered by CI on release/stable-with-patches | ||
git checkout release/stable-with-patches | ||
MAIN_COMMIT_BASE=$(git log -n 1 --pretty=%H main) # Get the latest main commit integrated | ||
BUILD_TAG="stable-$(date +%Y%m%d)-$(git rev-parse --short HEAD)-main-${MAIN_COMMIT_BASE:0:7}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tagging with a date isn't super useful if we want all our other repos to not require changes when we update our images.
In other words, I expect other repos to just use :latest-stable
whenever they reference openhands. So an update to latest-stable applies everywhere.
|
||
**The CI/CD pipeline for `release/stable-with-patches` handles the build, tag, and push process automatically.** Configuration updates might be manual or automated depending on your deployment strategy. | ||
|
||
## 3. Upstream Synchronization and Integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned above, I envision this section to be an openhands task that runs regularly to generate a PR that we review.
|
||
**e. Testing:** | ||
|
||
After rebasing `release/stable-with-patches`, thoroughly test your fork. Run linters, unit tests, integration tests, and perform manual checks focusing on areas affected by both upstream changes and your custom patches. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use acme-inc
as a testbed? Have it ways point to release/next-with-patches
that gets automatically updated and then as part of review we can confirm key flows work there?
|
||
All custom development and integration of external patches happen relative to the `release/stable-with-patches` branch. | ||
|
||
**a. Create Feature Branches:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, workflow-related changes are a pain to test. We might want another test app acme-openhands
that always tracks release/experimental
or something that we can openly push to in order to quickly test flows.
* **External PR Merged Upstream:** If an external PR you integrated via a feature branch is merged into `upstream/main`, the changes will eventually be incorporated into `release/stable-with-patches` when it's rebased onto `main` (Section 3d). You can simply delete your temporary feature branch (`feature/try-upstream-pr-456`). The rebase of `release/stable-with-patches` might require conflict resolution if your temporary integration conflicts with the official merge. | ||
* **Custom Feature No Longer Needed:** If a custom feature merged into `release/stable-with-patches` needs to be reverted, use `git revert` on the `release/stable-with-patches` branch to undo the merge commit (or the specific commits if squashed). This creates a new commit that undoes the changes. | ||
|
||
## 5. Workflow Summary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this section is meaningful to discuss, most of the above questions will have answers that change this significantly.
End-user friendly description of the problem this fixes or functionality that this introduces.
Give a summary of what the PR does, explaining any non-trivial design decisions.
Link of any specific issues this addresses.