An Issue with CVS Workflow?
- Amol mentioned a problem with a new directory (`StRoot/RTS/src/TRG_FCS`) not checked out by default in the local area (`cvs update` vs `cvs checkout`) - The new directory is in the CVS repository on the main branch and transferred to the Git repository as expected - So, anyone checking out a fresh copy of the parent directory from either Git or CVS would have a local copy - The fact that the production team is updating there local files instead of checking out seems very specific to their workflow - The need for a private communication cannot qualify the workflow as reproducible
Introduction
- Thanks to the Production Team and Amol's tests we now have confidence in the STAR libraries built from the Git repo - It is time to make STAR presence on GitHub official: - https://github.com/star-bnl — The STAR organization - https://github.com/star-bnl/star-sw — The primary repository with the code migrated from CVS - Important to note for all collaborators - A GitHub account is required for submitting code changes, receiving notifications, and participating in discussions - Notifications sent to your email are highly customizable. But the easiest way is to click the "Watch" button in a repository and select a watch option from the drop-down menu - Tons of documentation and guides for Git and GitHub available. Online resources cover everything from very basic and common operations to advanced topics: - https://docs.github.com/ — Documentation for GitHub interface and features - https://www.atlassian.com/git/tutorials/ — Tutorials, e.g. workflows explained - https://git-scm.com/book/ — Reference for all Git CLI utilities
Policies: General Ideas
- With technical details sorted out it is time to think about the rules - GitHub provides the ability to configure and enforce certain workflows for collaborative development - We suggest to start with stricter policies and relax them later if/when find appropriate - **Protect all branches:** Disable force pushes and prevent branches from being deleted - **Maintain linear history:** Prevent merge commits from being pushed to branches - Linear history is usually easier for humans to understand and debug - Consistent with historical development in CVS - **Require pull requests on GitHub** as the only way to merge commits onto remote branch - Direct pushes to remote branches are disabled for all collaborators - PRs encourage better documentation of proposed changes (via GitHub interface) - *Require status checks to pass before merging (not implemented)* - *Currently, we don't have any automatic status checks...* - Require branches to be up to date before merging
Policies: Code Reviews
- **Require pull request reviews before merging** - All pull requests must have a required number of approving reviews - We suggest two required approving reviews before a pull request can be merged - GitHub suggests reviewers based on historical code changes. It is also possible to request a review from someone other than a suggested person - Other collaborators can still review/comment on a submitted pull request - Require reviews from code owners/maintainers - Similar to `CVSROOT/avail` we create a file mapping paths/subdirectories to specific people whose approval is required - A `CODEOWNERS` file in a GitHub repository defines individuals responsible for code in a repository - It is not very clear if a code owner can approve their own PR... This might be critical for merging quick fixes - In cases when reviewers cannot be easily identified the Infrastructure Team will step in
Quick Recap
- Maintenance of the Git repository will be the responsibility of Infrastructure Team - In general, the gist of the proposed workflow is not very different from the historical one - The major difference is in the requirements to review submitted changes by code owners/maintainers - Taking care of documentation on agreed workflows, specific commands, links to external resources, examples, etc. - It is generally a good idea to keep the documentation close to the code. Ideally, it should be in the same repository and evolve with the code - Before `star-sw` is made writable, the following support repository can be used to share and discuss documentation: https://github.com/star-bnl/star-git-tools - It can be used to build the `CODEOWNERS` file for `star-sw`
Proposed Timeline (Xin)
- **March 1** — Make announcement to the collaboration about the intention for this transition - **March 8** — Presentation at the Collaboration meeting plenary session - **March 15** — Deadline for feedback from the collaboration - **March 17/24** — Discussion in the S&C management to respond to the feedbacks - **March 29** — Responses to feedback and final call for the transition - **April 1** — The switch day
- A list of action items for the "switch day" - In `CVSROOT/avail` disable write access to top-level directories transferred to Git for all users - Tag the code in CVS with next `SL21x` tag - Run sync jobs and stop the crontab job synchronizing Git and CVS - Push `CODEOWNERS` file to `star-sw`