indygreg / indygreg/PyOxidizer
Pull-request workflow
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 6.2k
- Forks
- 256
- PR merge metrics
- No merged PRs in 30d
Description
Current process
Currently it appears that pull requests are usually merged via cherry-pick. Some examples:
- #369's 3f15dda5bc5b52c4e713c00c95503e126d008873 was cherry-picked into 78cc3f2339f674f7a7649d781e13f62f73884fa2
- #354's commits were squashed and cherry-picked as e1a644e25be70753106ef7c2d34a5003cfbcc5fd
- #333's cfd6835473f37cdd79f348a454e44d7b27f0575a was cherry-picked into 1eedf3e391493d88cdc53f65174aa24293ecd6b5
Drawbacks
This process comes with a few downsides.
- A quick glance at #369 or #354 make them look like they were closed without being merged.
- If you navigate to #369's commit page or to 3f15dda5bc5b52c4e713c00c95503e126d008873 itself, it looks like the commit has been orphaned and not used.


If you want to know what version a bug's fix is available in, it's nice to be able to look at the commit and see what tags and branches contain it.

- The contributor of the PR loses the ability to influence to the commit messages, squashing process, etc.
- It's difficult—or at least not obvious how—to get GitHub (or Git itself) to show a diff between the PR and the final, cherry-picked commit. This means that the contributor has a hard time seeing if the code she wrote is actually what made it into PyOxidizer.
Proposed process
Here's my proposed contributor workflow:
- The contributor of a PR and the reviewers iterate on the review until the code is ready to be merged as is, apart from potentially needing to be squashed and rebased.
- For especially complicated PRs, once the reviewers are satisfied with the code itself, reviewers can ask the contributor to squash, rebase onto master, rewrite the commit message, and
git push --force-with-lease. This can include revisions to the commit message to conform to a project-preferred style.
- For especially complicated PRs, once the reviewers are satisfied with the code itself, reviewers can ask the contributor to squash, rebase onto master, rewrite the commit message, and
- The committer (i.e., @indygreg) squashes the PR locally, rebases onto
main, andgit push --force-with-lease.See django/django#14069 for an example of how this looks. Most importantly, this creates a "force-pushed" link that shows what changed during the squash/rebase.

- Once all the PR checks/CI tests pass, the committer hits GitHub's "Merge" button. Alternatively, the committer could use the "Rebase and merge" button in case there were commits to
mainsince CI started running after the force-push.

This workflow should mitigate all of the drawbacks listed above while still allowing PyOxidizer to maintain a linear Git history and clear commit messages. Personally, I don't view the linear Git history as essential, which would allow avoiding the squash/rebase steps. Anyway, there are a number of alternatives to my proposal that also solve the drawbacks to the current system. This ticket seems like a reasonable place to discuss those if anyone wants to do things differently.
Ultimately, whether and how to change process is entirely up to @indygreg as the owner.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
This issue is a maintainer process proposal rather than a code task, and it names no files, tests, or entry points. Review the Current process and Proposed process sections first; done would require the project owner to choose and adopt a pull-request workflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, github
- Domain
- developer-experience, release
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100